Re: [QUESTION] How to fix the race of "mdadm --add" and "mdadm mdadm --incremental --export"

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Mon, 20 Mar 2023 16:36:44 +0100
Martin Wilck <mwilck@suse.com> wrote:

> On Thu, 2023-03-16 at 11:44 +0100, Mariusz Tkaczyk wrote:
> > On Wed, 15 Mar 2023 16:01:26 +0100
> > Martin Wilck <mwilck@suse.com> wrote:  
> > > 
> > > Normally this is what you want to happen if a change uevent for a
> > > MD
> > > member device is processed.
> > > 
> > > The case you're looking at is the exception, as another instance of
> > > mdamn is handling the device right at this point in time.
> > > 
> > > Martin
> > >   
> > Hi,
> > Code snipped from Incremental, devname seems to be our disk:
> > 
> >         /* 4/ Check if array exists.
> >          */
> >         if (map_lock(&map))
> >                 pr_err("failed to get exclusive lock on mapfile\n");
> >         /* Now check we can get O_EXCL.  If not, probably "mdadm -A"
> > has
> >          * taken over
> >          */
> >         dfd = dev_open(devname, O_RDONLY|O_EXCL);
> > 
> > The map_lock in --add should resolve your issue. It seems to be
> > simplest
> > option. I we cannot lock udev effectively then it seems to be the
> > best one.
> > We need to control adding the device because external metadata is
> > quite
> > different and for example in IMSM the initial metadata doesn't point
> > to
> > container wanted by user. Hope it clarifies all objections here.
> > 
> > I don't see any blocker from using locking mechanism for --add.  
> 
> 
> AFAICS it would only help if the code snipped above did not only
> pr_err() but exit if it can't get an exclusive lock.
> 
> Martin
> 

Indeed. I missed that... Thanks for catching. My idea is no longer valid.

Mariusz