[LU-2349] osp_mds_ost_orig_logops should only be initialized during module init.

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.4.0
• Type: Bug
• Priority: Minor
• Reporter: Di Wang
• Assignee: Di Wang
• Resolution: Fixed
• Severity: 3

Description

Because osp_mds_ost_orig_logops and changelog_orig_logops will be only shared for all of the ctxt for all OSPs/MDDs, if we keep changing that during obd_setup process, the ctxt for other obd might be impacted. Specially, loc_logops.lop_add and loc_logops.lop_declare_add might become zero sometimes(especially for DNE). So we should move the ops assignment to module init.

Comments

[Di Wang]

http://review.whamcloud.com/#change,4608

[Mikhail Pershin]

this is not quite right, in ctxt the only basic device operations are set, i.e. osd_ops or client_ops and only they are shared but that is safe. The lop_add and lop_declare_add are always NULL here and set per-handle of particular catalog llog when it is opened. So all you need to do is just set them when llog catalog of each MDT is opened, I suppose they are not shared, right?

[Di Wang]

Hmm, I am not sure I follow your point here. I did not mean to set to these operations per ctxt. The reason I assign logops in module init is that the orignal implementation will change these logops during setup process, but this ops will be shared by all of the OSP on the node. And also if lop_add/declare_add will be always llog_cat_add_rec/xxx_declare_add_rec, what is the point of resetting them every time?

[Mikhail Pershin]

Original implementation is not changing logops but initialize them, e.g. set ctxt logops to the osd logops. If this is shared now then it is OK to move that initialization to the module init, but not lgh_logops which are llog_add/declare_add, they are not shared as log_handle is unique.

There are two sets of log_ops - one in ctxt which is set of operations defined by bottom device type - osd or client or some other. Another set is in llog_handle - lgh_logops, it is initialized in addition to common set from ctxt, adding llog_type operations like llog_add for catalogs because only catalogs need it. Note that upon llog init the lgh_logops are initialized from common set in ctxt plus catalog operations like llog_cat_add_rec only if needed. That helps to avoid problems like plain llog has operations of catalog llog and so on. All llog operations with llog_handle are using llog_ops from handle not from ctxt, so it we can set individual methods for any llog type at any layer, not all of them might want to use llog_cat_add_rec as llog_add.

[Di Wang]

Hmm, you mean llog_add/declare_add should not be put to somewhere else instead of ctxt logops, i.e. they should not be shared by all osp/osd?

[Mikhail Pershin]

No, I mean log_add/declare_add are in llog_handle logops now but not in ctxt logops. Your patch moves them to the ctxt logops which is not good because these ops are not the same for every device but related to particular llog. E.g. plain logs don't have them, catalogs have but it is not necessary llog_cat_add_rec(), for example some layer may define own methods for llog_add. That is why it should be assigned to the loghandle->lgh_logops but not be in ctxt logops.

[Mikhail Pershin]

Di, just to be clear - my intention was to don't mix common operations like osd logops and llog_add which makes sense only for catalogs. E.g. having it is ctxt logops means all plain llogs will have llog_add defined too. That is why I assign llog_add as part of llog catalog init process. Meanwhile that is not strict requirement so I can agree with your proposal too with proper comment about we may have llog_cat_add_rec/declare is ctxt ops because it is used by all catalogs in that context.

[Andreas Dilger]

Patch landed for 2.4.0.