All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Harald Hoyer <harald@redhat.com>
Cc: Jes.Sorensen@redhat.com, linux-raid@vger.kernel.org,
	Kay Sievers <kay@vrfy.org>
Subject: Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation
Date: Mon, 29 Apr 2013 16:53:50 +1000	[thread overview]
Message-ID: <20130429165350.32329f90@notabene.brown> (raw)
In-Reply-To: <517E13FF.6050308@redhat.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 29 Apr 2013 08:32:31 +0200 Harald Hoyer <harald@redhat.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/29/2013 08:11 AM, NeilBrown wrote:
> > On Mon, 29 Apr 2013 07:33:21 +0200 Harald Hoyer <harald@redhat.com> wrote:
> > 
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> > 
> >> On 04/29/2013 02:57 AM, NeilBrown wrote:
> >>> On Thu, 11 Apr 2013 15:18:33 +0200 Jes.Sorensen@redhat.com wrote:
> >>> 
> >>>> From: Harald Hoyer <harald@redhat.com>
> >>>> 
> >>>> This does not trigger the udev inotify twice and saves a lot of blk
> >>>> I/O for the raid members.
> >>>> 
> >>>> Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=947815
> >>>> 
> >>>> Signed-off-by: Harald Hoyer <harald@redhat.com> Signed-off-by: Jes 
> >>>> Sorensen <Jes.Sorensen@redhat.com>
> >>> 
> >>> (Sorry for delays.  Thanks for reminders).
> >>> 
> >>> That patch seems to make sense, but the description above is awfully
> >>> thin.
> >>> 
> >>> Why is double-open a problem exactly?  What does it make udev do?  And
> >>> how does that related to ID_FS_TYPE being wrong as mentioned in the
> >>> bugzilla entry.
> >>> 
> >>> NeilBrown
> >>> 
> > 
> >> udevd with watch enabled (inotify on /dev/sd*) gets triggered on close(),
> >> when you opened it writeable. So, if you double open() and udev wakes up
> >> from the first close(), not all information are written to disk yet, it
> >> will not get the ID_FS_TYPE.
> > 
> >> Seems like the second close() does not trigger an inotify sometimes, so
> >> it is missing afterwards all the time.
> > 
> >> Watch via inotify is just a lazy workaround, so we don't have to modify
> >> every tool to emit a "change" uevent, after they changed the disk.
> > 
> > So udev have a "lazy workaround" so that other programs don't need to
> > trigger a change, and as a result, I need to add some special code to
> > mdadm. Doesn't seem like I'm getting any advantage out of this laziness.
> > 
> > How about when udev gets an inotify for a block device, it first checks 
> > that it can open it O_EXCL.  If not, it doesn't generate the change event. 
> > That seems like the laziest option to me :-)
> 
> We cannot open with O_EXCL, because the device can be mounted, and O_EXCL
> would fail there.
> 

If the device is mounted, why would you want udev to be doing anything to it?

I assumed this was for things like "mkfs" so that as soon as you mkfs a
filesystem udev could tell udisks to immediately mount it...  though I'm not
sure this is a good idea.

I'm probably missing something important: what is the particular use case for
udev mapping a close-after-write to a change event?

Thanks,
NeilBrown
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIVAwUBUX4Y/jnsnt1WYoG5AQKvwA//XoMDQJrPb+Iu3X8+IgomwT0PX58Pqhu6
gTQ6o1LGBOX7ir3ddFn0nGN7dRX05dngVdbOZkVqrXwJePm+uE8E9+tFLsb6VCwM
77mZue44eNhLpgVQ3C7HMt1LWBy8traDBq0PgJYeNu6yOXaQTh+mINrJ2w1c2dpe
hQnvYGIwYOj4SYDj9IbTubCwoMn9f6B/kHFQ7xSjkeLQDrKtE3yIm0RfYHtUIXgz
vF9Zrtu9HBl+C4zMqLt8qPtNUGWymeGApPNYU2n1RAeKqBKL/C88tj7Rfj0TR5qp
VssHPJ+utGvO9sZADrWFm4cHNA3wHuhXYCQZFLjDRSijILXe/vGcIk0ooEeqmLSj
G/n1dqMsfOnvO6+I86kHlJTbouhkpdSTKQi+0+URctjecN6FutsEhCWkyOtOsUWK
v2XFdwUmx64JZ6h3StMjpV70lEaSZohFvgnigyT75rC/fL5jPoS0ss28o7dr1urN
lUDiV4Mbrx5xVDaixe96OeKP1ZHxtXKhCV7/6rmnRdFN6DQWNrWasiFz/LOVafjn
ndb6bc5IMLl7b2++JY3ySS6j1MKEnFoqXiO7RqZk20ZPWIkj/naCfnk1tOUx6tA5
U1O3tquFn3KYGKRl/idl0Xhh0NxnDCw5gCpBySFf74N10Y0VCYKpkAHsABsBO/uV
L+TDpWZQJKE=
=dYDW
-----END PGP SIGNATURE-----

  reply	other threads:[~2013-04-29  6:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-11 13:18 [PATCH 0/1] Reduce unnecessary opens of raid members Jes.Sorensen
2013-04-11 13:18 ` [PATCH 1/1] prevent double open(O_RDWR) on raid creation Jes.Sorensen
2013-04-29  0:57   ` NeilBrown
2013-04-29  5:33     ` Harald Hoyer
2013-04-29  6:11       ` NeilBrown
2013-04-29  6:32         ` Harald Hoyer
2013-04-29  6:53           ` NeilBrown [this message]
2013-04-29  8:34             ` Harald Hoyer
2013-04-29  8:40             ` Harald Hoyer
2013-04-29  8:45               ` Harald Hoyer
2013-04-29  8:54                 ` Harald Hoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130429165350.32329f90@notabene.brown \
    --to=neilb@suse.de \
    --cc=Jes.Sorensen@redhat.com \
    --cc=harald@redhat.com \
    --cc=kay@vrfy.org \
    --cc=linux-raid@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.