[dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[Benjamin Marzinski]

On Mon, Nov 28, 2016 at 11:05:29AM +0100, Hannes Reinecke wrote:
> Hi Junhui,
> 
> On 11/28/2016 03:19 AM, tang.junhui at zte.com.cn wrote:
> > Hello Christophe, Ben, Hannes, Martin, Bart,
> > I am a member of host-side software development team of ZXUSP storage
> > project in ZTE Corporation. Facing the market demand, our team decides to write
> > code to promote multipath efficiency next month. The whole idea is in the mail
> > below. We hope to participate in and make progress with the open source community,
> > so any suggestion and comment would be welcome.
> > 
> Thank you for looking into it.
> This is one area which definitely needs improvement, so your
> contribution would be most welcome.
> 
> > 
> > ------------------------------------------------------------------------------------------------------------------------------
> > 
> > ------------------------------------------------------------------------------------------------------------------------------
> > 
> > 1.        Problem
> > In these scenarios, multipath processing efficiency is low:
> > 1) Many paths exist in each multipath device,
> > 2) Devices addition or deletion during iSCSI login/logout or FC link
> > up/down.
> > 
> > 2.        Reasons
> > Multipath process uevents one by one, and each one also produce a new dm
> > addition change or deletionuevent to increased system resource consumption,
> > actually most of these uevents have no sense at all.
> > 
> [ .. ]
> > In list_merger_uevents(&uevq_tmp), each node is traversed from the
> > latest to the oldest,
> > and the older node with the same WWID and uevent type(e.g. add) would be
> > moved to
> > the merger_node list of the later node. If a deletion uevent node
> > occurred, other older
> > uevent nodes about this device would be filtered(Thanks to Martin’s idea).
> > 
> > After above processing, attention must be paid to that the parameter
> > “struct uevent * uev” is not a single uevent any more in and after
> > uev_trigger(), code
> > need to be modified to process batch uevents in uev_add_path() and so on.
> > 
> To handle this properly you need to modify uev_add_path(); basically you
> need to treat 'uev_add_path()' and 'ev_add_path()' as two _distinct_
> events, where the former processes uevents and add the path to an
> internal list (->pathvec would be ideally suited here).
> 'ev_add_path()' then would need to be called once all uevents are
> processed, and would be processing all elements in ->pathvec, creating
> the resulting multipath map in one go.
> 
> Actually not a bad idea.
> 
> Tricky bit would be to figure out _when_ to stop uevent processing and
> calling 'ev_add_path'.
> I guess we'd need to make 'ev_add_path' running in a separate thread;
> that way we'd be putting pressure off the uevent processing thread and
> the whole thing should be running far smoother.

I still think that we don't need to force a wait here.
uevent_dispatch() already pulls off all the queued events in a batch. We
could just merge over the batch that we are given.  This has the nice
property that when uevents aren't coming in a storm, we quickly process
the next event, and when they are, the further multipathd gets behind, the
larger the list it will merge over.
 
> And looking at it closer, when you move 'ev_add_path' into a separate
> thread, and separating out 'uev_add_path' you _already_ have your event
> merging implemented.

Like I mentioned before, I'm not sure what a seperate thread will buy us
here, since we can't create multipath devices while paths might be
getting deleted. I do think your earlier point that we don't need any
locking until the paths are added to the pathvec is important.  Instead
of going a path at a time, getting the path's information and then
adding it to the pathvec, we could get the information on all the
outstanding paths and then add them all at once, and then call the
map creation code (basically the work that ev_add_path does) on all the
paths.

> With much less effort then trying to merge things at the uevent level.

Yes. I'd much rather deal with the uevents as they acutally are, and
have uev_add_path (possibly now called uev_add_paths) do the work.

> I'd be willing to look into it; please keep me informed about any
> progress here.
> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		   Teamlead Storage & Networking
> hare at suse.de			               +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)