* [PATCH 0/1] Reduce unnecessary opens of raid members @ 2013-04-11 13:18 Jes.Sorensen 2013-04-11 13:18 ` [PATCH 1/1] prevent double open(O_RDWR) on raid creation Jes.Sorensen 0 siblings, 1 reply; 11+ messages in thread From: Jes.Sorensen @ 2013-04-11 13:18 UTC (permalink / raw) To: neilb; +Cc: harald, linux-raid From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi, I am posting this on Harald's request. This avoids unncessary double opens of raid members and also has the side effect of not triggering udev notify twice. Jes Harald Hoyer (1): prevent double open(O_RDWR) on raid creation Kill.c | 28 +++++++++++++++++----------- Manage.c | 2 +- mdadm.c | 4 ++-- mdadm.h | 2 +- super-ddf.c | 2 +- super-intel.c | 2 +- super0.c | 2 +- super1.c | 2 +- 8 files changed, 25 insertions(+), 19 deletions(-) -- 1.8.1.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-11 13:18 [PATCH 0/1] Reduce unnecessary opens of raid members Jes.Sorensen @ 2013-04-11 13:18 ` Jes.Sorensen 2013-04-29 0:57 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Jes.Sorensen @ 2013-04-11 13:18 UTC (permalink / raw) To: neilb; +Cc: harald, linux-raid 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> --- Kill.c | 28 +++++++++++++++++----------- Manage.c | 2 +- mdadm.c | 4 ++-- mdadm.h | 2 +- super-ddf.c | 2 +- super-intel.c | 2 +- super0.c | 2 +- super1.c | 2 +- 8 files changed, 25 insertions(+), 19 deletions(-) diff --git a/Kill.c b/Kill.c index f2fdb85..b5c93ae 100644 --- a/Kill.c +++ b/Kill.c @@ -29,7 +29,7 @@ #include "md_u.h" #include "md_p.h" -int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) +int Kill(char *dev, int oldfd, struct supertype *st, int force, int verbose, int noexcl) { /* * Nothing fancy about Kill. It just zeroes out a superblock @@ -42,21 +42,26 @@ int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) int fd, rv = 0; - if (force) - noexcl = 1; - fd = open(dev, O_RDWR|(noexcl ? 0 : O_EXCL)); - if (fd < 0) { - if (verbose >= 0) - pr_err("Couldn't open %s for write - not zeroing\n", - dev); - return 2; + if (oldfd != -1) { + fd = oldfd; + } else { + if (force) + noexcl = 1; + fd = open(dev, O_RDWR|(noexcl ? 0 : O_EXCL)); + if (fd < 0) { + if (verbose >= 0) + pr_err("Couldn't open %s for write - not zeroing\n", + dev); + return 2; + } } if (st == NULL) st = guess_super(fd); if (st == NULL || st->ss->init_super == NULL) { if (verbose >= 0) pr_err("Unrecognised md component device - %s\n", dev); - close(fd); + if (oldfd == -1) + close(fd); return 2; } st->ignore_hw_compat = 1; @@ -76,7 +81,8 @@ int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) rv = 0; } } - close(fd); + if (oldfd == -1) + close(fd); return rv; } diff --git a/Manage.c b/Manage.c index 6267c0c..509e921 100644 --- a/Manage.c +++ b/Manage.c @@ -785,7 +785,7 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv, return -1; } - Kill(dv->devname, NULL, 0, -1, 0); + Kill(dv->devname, -1, NULL, 0, -1, 0); dfd = dev_open(dv->devname, O_RDWR | O_EXCL|O_DIRECT); if (mdmon_running(tst->container_devnm)) tst->update_tail = &tst->updates; diff --git a/mdadm.c b/mdadm.c index 214afa3..d68ee96 100644 --- a/mdadm.c +++ b/mdadm.c @@ -1739,11 +1739,11 @@ static int misc_list(struct mddev_dev *devlist, continue; case KillOpt: /* Zero superblock */ if (ss) - rv |= Kill(dv->devname, ss, c->force, c->verbose,0); + rv |= Kill(dv->devname, -1, ss, c->force, c->verbose, 0); else { int v = c->verbose; do { - rv |= Kill(dv->devname, NULL, c->force, v, 0); + rv |= Kill(dv->devname, -1, NULL, c->force, v, 0); v = -1; } while (rv == 0); rv &= ~2; diff --git a/mdadm.h b/mdadm.h index c7b5183..e55dec5 100644 --- a/mdadm.h +++ b/mdadm.h @@ -1156,7 +1156,7 @@ extern int Monitor(struct mddev_dev *devlist, int dosyslog, char *pidfile, int increments, int share); -extern int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl); +extern int Kill(char *dev, int oldfd, struct supertype *st, int force, int verbose, int noexcl); extern int Kill_subarray(char *dev, char *subarray, int verbose); extern int Update_subarray(char *dev, char *subarray, char *update, struct mddev_ident *ident, int quiet); extern int Wait(char *dev); diff --git a/super-ddf.c b/super-ddf.c index c5f6f5c..a88699c 100644 --- a/super-ddf.c +++ b/super-ddf.c @@ -2597,7 +2597,7 @@ static int write_init_super_ddf(struct supertype *st) } else { struct dl *d; for (d = ddf->dlist; d; d=d->next) - while (Kill(d->devname, NULL, 0, -1, 1) == 0); + while (Kill(d->devname, d->fd, NULL, 0, -1, 1) == 0); return __write_init_super_ddf(st); } } diff --git a/super-intel.c b/super-intel.c index 24016b7..743a7fc 100644 --- a/super-intel.c +++ b/super-intel.c @@ -5221,7 +5221,7 @@ static int write_init_super_imsm(struct supertype *st) } else { struct dl *d; for (d = super->disks; d; d = d->next) - Kill(d->devname, NULL, 0, -1, 1); + Kill(d->devname, d->fd, NULL, 0, -1, 1); return write_super_imsm(st, 1); } } diff --git a/super0.c b/super0.c index 1f4dc75..57b549e 100644 --- a/super0.c +++ b/super0.c @@ -779,7 +779,7 @@ static int write_init_super0(struct supertype *st) continue; if (di->fd == -1) continue; - while (Kill(di->devname, NULL, 0, -1, 1) == 0) + while (Kill(di->devname, di->fd, NULL, 0, -1, 1) == 0) ; sb->disks[di->disk.number].state &= ~(1<<MD_DISK_FAULTY); diff --git a/super1.c b/super1.c index d0f1d5f..e3eeb80 100644 --- a/super1.c +++ b/super1.c @@ -1333,7 +1333,7 @@ static int write_init_super1(struct supertype *st) if (di->fd < 0) continue; - while (Kill(di->devname, NULL, 0, -1, 1) == 0) + while (Kill(di->devname, di->fd, NULL, 0, -1, 1) == 0) ; sb->dev_number = __cpu_to_le32(di->disk.number); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 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 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2013-04-29 0:57 UTC (permalink / raw) To: Jes.Sorensen; +Cc: harald, linux-raid [-- Attachment #1: Type: text/plain, Size: 6002 bytes --] 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 > --- > Kill.c | 28 +++++++++++++++++----------- > Manage.c | 2 +- > mdadm.c | 4 ++-- > mdadm.h | 2 +- > super-ddf.c | 2 +- > super-intel.c | 2 +- > super0.c | 2 +- > super1.c | 2 +- > 8 files changed, 25 insertions(+), 19 deletions(-) > > diff --git a/Kill.c b/Kill.c > index f2fdb85..b5c93ae 100644 > --- a/Kill.c > +++ b/Kill.c > @@ -29,7 +29,7 @@ > #include "md_u.h" > #include "md_p.h" > > -int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) > +int Kill(char *dev, int oldfd, struct supertype *st, int force, int verbose, int noexcl) > { > /* > * Nothing fancy about Kill. It just zeroes out a superblock > @@ -42,21 +42,26 @@ int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) > > int fd, rv = 0; > > - if (force) > - noexcl = 1; > - fd = open(dev, O_RDWR|(noexcl ? 0 : O_EXCL)); > - if (fd < 0) { > - if (verbose >= 0) > - pr_err("Couldn't open %s for write - not zeroing\n", > - dev); > - return 2; > + if (oldfd != -1) { > + fd = oldfd; > + } else { > + if (force) > + noexcl = 1; > + fd = open(dev, O_RDWR|(noexcl ? 0 : O_EXCL)); > + if (fd < 0) { > + if (verbose >= 0) > + pr_err("Couldn't open %s for write - not zeroing\n", > + dev); > + return 2; > + } > } > if (st == NULL) > st = guess_super(fd); > if (st == NULL || st->ss->init_super == NULL) { > if (verbose >= 0) > pr_err("Unrecognised md component device - %s\n", dev); > - close(fd); > + if (oldfd == -1) > + close(fd); > return 2; > } > st->ignore_hw_compat = 1; > @@ -76,7 +81,8 @@ int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl) > rv = 0; > } > } > - close(fd); > + if (oldfd == -1) > + close(fd); > return rv; > } > > diff --git a/Manage.c b/Manage.c > index 6267c0c..509e921 100644 > --- a/Manage.c > +++ b/Manage.c > @@ -785,7 +785,7 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv, > return -1; > } > > - Kill(dv->devname, NULL, 0, -1, 0); > + Kill(dv->devname, -1, NULL, 0, -1, 0); > dfd = dev_open(dv->devname, O_RDWR | O_EXCL|O_DIRECT); > if (mdmon_running(tst->container_devnm)) > tst->update_tail = &tst->updates; > diff --git a/mdadm.c b/mdadm.c > index 214afa3..d68ee96 100644 > --- a/mdadm.c > +++ b/mdadm.c > @@ -1739,11 +1739,11 @@ static int misc_list(struct mddev_dev *devlist, > continue; > case KillOpt: /* Zero superblock */ > if (ss) > - rv |= Kill(dv->devname, ss, c->force, c->verbose,0); > + rv |= Kill(dv->devname, -1, ss, c->force, c->verbose, 0); > else { > int v = c->verbose; > do { > - rv |= Kill(dv->devname, NULL, c->force, v, 0); > + rv |= Kill(dv->devname, -1, NULL, c->force, v, 0); > v = -1; > } while (rv == 0); > rv &= ~2; > diff --git a/mdadm.h b/mdadm.h > index c7b5183..e55dec5 100644 > --- a/mdadm.h > +++ b/mdadm.h > @@ -1156,7 +1156,7 @@ extern int Monitor(struct mddev_dev *devlist, > int dosyslog, char *pidfile, int increments, > int share); > > -extern int Kill(char *dev, struct supertype *st, int force, int verbose, int noexcl); > +extern int Kill(char *dev, int oldfd, struct supertype *st, int force, int verbose, int noexcl); > extern int Kill_subarray(char *dev, char *subarray, int verbose); > extern int Update_subarray(char *dev, char *subarray, char *update, struct mddev_ident *ident, int quiet); > extern int Wait(char *dev); > diff --git a/super-ddf.c b/super-ddf.c > index c5f6f5c..a88699c 100644 > --- a/super-ddf.c > +++ b/super-ddf.c > @@ -2597,7 +2597,7 @@ static int write_init_super_ddf(struct supertype *st) > } else { > struct dl *d; > for (d = ddf->dlist; d; d=d->next) > - while (Kill(d->devname, NULL, 0, -1, 1) == 0); > + while (Kill(d->devname, d->fd, NULL, 0, -1, 1) == 0); > return __write_init_super_ddf(st); > } > } > diff --git a/super-intel.c b/super-intel.c > index 24016b7..743a7fc 100644 > --- a/super-intel.c > +++ b/super-intel.c > @@ -5221,7 +5221,7 @@ static int write_init_super_imsm(struct supertype *st) > } else { > struct dl *d; > for (d = super->disks; d; d = d->next) > - Kill(d->devname, NULL, 0, -1, 1); > + Kill(d->devname, d->fd, NULL, 0, -1, 1); > return write_super_imsm(st, 1); > } > } > diff --git a/super0.c b/super0.c > index 1f4dc75..57b549e 100644 > --- a/super0.c > +++ b/super0.c > @@ -779,7 +779,7 @@ static int write_init_super0(struct supertype *st) > continue; > if (di->fd == -1) > continue; > - while (Kill(di->devname, NULL, 0, -1, 1) == 0) > + while (Kill(di->devname, di->fd, NULL, 0, -1, 1) == 0) > ; > > sb->disks[di->disk.number].state &= ~(1<<MD_DISK_FAULTY); > diff --git a/super1.c b/super1.c > index d0f1d5f..e3eeb80 100644 > --- a/super1.c > +++ b/super1.c > @@ -1333,7 +1333,7 @@ static int write_init_super1(struct supertype *st) > if (di->fd < 0) > continue; > > - while (Kill(di->devname, NULL, 0, -1, 1) == 0) > + while (Kill(di->devname, di->fd, NULL, 0, -1, 1) == 0) > ; > > sb->dev_number = __cpu_to_le32(di->disk.number); [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 0:57 ` NeilBrown @ 2013-04-29 5:33 ` Harald Hoyer 2013-04-29 6:11 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 5:33 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid -----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. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfgYhAAoJEANOs3ABTfJw0uUQALrm0pEjlLd6XgojMTQ6xJGy y98MVcobi10O/WJyg3HV1RqjnYNu7wfpp+lFIzKRmE/sxIBj8X9ATFfjaopCGWYC /MPGsdehrCpGPPOZBlt47vTdoaKWB5meKsBm3X1I0AhA+uOxgeV2qfaijoOkHeim a4RbIUoOJIjIyvbdKCuVbs8mqcr62eMUiZBDPv/b3FcBtOOjYkWVZU14ylqujNtM WzE+soKyd6L70DvPWVY2KzJ4/5bg/fRuvFcc464k88hAqa8U36FU6MzfTuT4K+ZH a4FYJtpdrggL+IZuG5XToNR5lpR/YW/B1UBhfCjItXbr1dhX3alk+Y3xZCWvpbRF FFwAA1GJfcB8UmKp3loBX0YH4gJ8h8d6EITE0Quj38VqG4MlCl89J6ClQZYgXsf8 ZCVDX+lomiQkEp5xYyC85hmwfwepibncqfqKef8+4ABc5xWswQr89gDFPVsFZUE/ PbHzCUlAkz8lvuRSNH6k54b7nFeGn116eJgO7sKESt4uygP0o1A6WpWZI+YAMMg5 CBkxrLYM/iERP7sf8kHr3Wd5EWJNTYm6UsJjVtWStGHuB7LNDo6qPBXxzf84Mkek 1fnIqBfl6QDQBcYb+02p2vGhcTA+P/byi+j+eFQmwV8g2gbkwxhV6/t0Sizj57tC SlUZAaWHeeNK9HDgoNJ0 =zkJO -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 5:33 ` Harald Hoyer @ 2013-04-29 6:11 ` NeilBrown 2013-04-29 6:32 ` Harald Hoyer 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2013-04-29 6:11 UTC (permalink / raw) To: Harald Hoyer; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 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 :-) NeilBrown > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.13 (GNU/Linux) > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iQIcBAEBAgAGBQJRfgYhAAoJEANOs3ABTfJw0uUQALrm0pEjlLd6XgojMTQ6xJGy > y98MVcobi10O/WJyg3HV1RqjnYNu7wfpp+lFIzKRmE/sxIBj8X9ATFfjaopCGWYC > /MPGsdehrCpGPPOZBlt47vTdoaKWB5meKsBm3X1I0AhA+uOxgeV2qfaijoOkHeim > a4RbIUoOJIjIyvbdKCuVbs8mqcr62eMUiZBDPv/b3FcBtOOjYkWVZU14ylqujNtM > WzE+soKyd6L70DvPWVY2KzJ4/5bg/fRuvFcc464k88hAqa8U36FU6MzfTuT4K+ZH > a4FYJtpdrggL+IZuG5XToNR5lpR/YW/B1UBhfCjItXbr1dhX3alk+Y3xZCWvpbRF > FFwAA1GJfcB8UmKp3loBX0YH4gJ8h8d6EITE0Quj38VqG4MlCl89J6ClQZYgXsf8 > ZCVDX+lomiQkEp5xYyC85hmwfwepibncqfqKef8+4ABc5xWswQr89gDFPVsFZUE/ > PbHzCUlAkz8lvuRSNH6k54b7nFeGn116eJgO7sKESt4uygP0o1A6WpWZI+YAMMg5 > CBkxrLYM/iERP7sf8kHr3Wd5EWJNTYm6UsJjVtWStGHuB7LNDo6qPBXxzf84Mkek > 1fnIqBfl6QDQBcYb+02p2vGhcTA+P/byi+j+eFQmwV8g2gbkwxhV6/t0Sizj57tC > SlUZAaWHeeNK9HDgoNJ0 > =zkJO > -----END PGP SIGNATURE----- -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIVAwUBUX4PDDnsnt1WYoG5AQInKA/+PyFlr+sO0cHRQRxG6EMi1Uy649MAf1u+ TOnvspRNilHs+ackxKOyT3ZNHYUwyeybVKlAvKuZ8ynObtUmE+E7OnGOcqLl28Ml As92W35y56qPl1czDhX/UDAZALjI0FrHcDBJLxef8bWLXCsEoUpPeqF5ddfmsvzL 7XR9Aiy7LHkBXQbPciYneR1kZHnuNF3zBFtt68bAnK4hV4128LuohM3v6FM4CU5q fiZpo+gqeKdnu7oev0Xh6inaP5MfupGvYA9UTTPiRV/Rg+9tv2Y/hSHtYE/mvWbH lLLAHq1e7Pcu1wCwXmlW0h2ph5rcs3rBOgQYpFWet65nbFk76q+NY01m0tqelVbv 6wt1JVyUxopHMy9FLypg+/cctDYLimgDFd2eG5B2YXL/lF5EBclM3PmhMDR6ITun wWGhburperRwKYVZszv9Eo8Teu1ed/1gDnD69XlOa8dOOXtj95WcBDGz6p+1hM/Q H7bWwXEiXreMrv3WtCvoghakM+ul9P3vZXo83Vwv7x7WDteEQ6ZYUe9CdVuM/k3r x+CYz7kjfkg2oZEZmN11pe3UoU+eCBR3FOm1xvbEiQt5SWZm8mRd7v7Y2qVI3e3C Q9M5rEJ8wws20JrRXHEtkOCX1FzeiYOi5calDWUzf+2AyY86NYv214SM98qWXgju AyUaIO7u0Mg= =zg9i -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 6:11 ` NeilBrown @ 2013-04-29 6:32 ` Harald Hoyer 2013-04-29 6:53 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 6:32 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----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. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfhP+AAoJEANOs3ABTfJwONcP+gOzhDDvWFG0AZdocgttk04o oV4mr2sde+NtuQMB8ac9v4Y0XP2HnpAYOVAJP7LALpkNHwUYGxtDDbqwWLGQWfPy xXt6Zq9jE/srwmbBTmCB5sROS5lw6c+M2ft4kza2PDDY+9xyTjxVqVf2tcRGa3Fh LIFNnKGk58qrgAI/46Muw0rbfMZOpOPuLNrI2aYwl/xmrXmQVXfGLIb+Rms9cdsY Duk/U05t/DxYupic/fAvkuDmuUzsyBAhpuixL/774m7J3yzf6sNIadtZgy3rMiym WNz+C8Ltx4d2H1nGmjkw9rKbmnWye15lLfsJGZQxvtt9LkWCPu6L9KbLKQyjjnc9 DBIj/6vID3JhxoLRE7H+zQXk7b9VX6sWAKPo6iFC4LcD8tESs01Jz0bV2kyMcUsZ 1HucYgcd2yzaaMBdEB9LleXd7SCtiBW74qTa/WgFK26notiyIGVWnfBdr9TYi1cB UJLbsa9XHadq/OW4qVEBWbDbkM6Wpwkc2W7oosh3klauifbn2PXguwp5oeT/CQ1y HORW36Xt9myAcZWY9iBjFRMwxJNztF89XYWOIzZ5j9fV1wHvCqYqNN+gtyt5vq97 N+Pn4pJ2Nuyygxu++aSv271JBga0YZCIGlIE38J1mJVgZlNAFXIGELs30NhYNhI2 GZ3eOMhawdR9k3NkQbMM =hDz6 -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 6:32 ` Harald Hoyer @ 2013-04-29 6:53 ` NeilBrown 2013-04-29 8:34 ` Harald Hoyer 2013-04-29 8:40 ` Harald Hoyer 0 siblings, 2 replies; 11+ messages in thread From: NeilBrown @ 2013-04-29 6:53 UTC (permalink / raw) To: Harald Hoyer; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----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----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 6:53 ` NeilBrown @ 2013-04-29 8:34 ` Harald Hoyer 2013-04-29 8:40 ` Harald Hoyer 1 sibling, 0 replies; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 8:34 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/29/2013 08:53 AM, NeilBrown wrote: > 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 Well, you would not recognize LABEL or UUID or MD UUID changes, if you don't call blkid after a mkfs or mdadm or lvm action. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfjCeAAoJEANOs3ABTfJwLQIP/iaKQFUYyE5tsw8zVVxw+ag6 QPhs08y8JM1nPNyC4hd2BKLwx687eXVIR7Kyfx8i9L2dEHLn2ry+ROTAbfn1UEr2 GjpD48wuNiohJKUZ5l8Wn3q8slboUImgHUkiZgtWII0pCk3vGo+/01WIWzSzDRlN PsI8TS4+wBIdZhfWMtzU65uXiZYoGpaURqIv+7xIgEN90mM45kE1FA/HJF8YZOBz H8CMI2pr+GUI/XCUjUYJK5m3y1sJCm5US5IsR4AqpQGlkU2G9pFQYRWFS4uYKOsF tjk7Fj8wFw51gz/dWGlKM9uK+iAFHVVorjFPdDJilSdswWP88JMrwlfAkivxbKSA 6JsKEOSLlkuW5gNRl6Flg2/hLBmlJYUx3nXzT4WZWgibD4mkKAmY1uB0gnzE2/lm YP7dEs5WVo5Rl3lM+Mv6vqDl6dJCvg60AU3u5srWwfqUau81p5rYIuqXJ5Da4GcU njz6NUwzMm2FI91INDDi/GJzwi1bVV6CeTNiz3LNjd2+yr2dt0AFBZWQ3k1c+/rO WJ8NhN6dW8JZEL4VN5yfEht4x+kVnxqdTedAm5NtPooSD3we2w/e5u5im3FfUkJN GX4+Et2sE5gk70Es/u2Ws1tLOdpxZsUiU/HBgS51+8KBIumj+Me3BLZpGkvqwNm/ vV2Fov6yJhM5E0/lYAhn =xI4C -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 6:53 ` NeilBrown 2013-04-29 8:34 ` Harald Hoyer @ 2013-04-29 8:40 ` Harald Hoyer 2013-04-29 8:45 ` Harald Hoyer 1 sibling, 1 reply; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 8:40 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/29/2013 08:53 AM, NeilBrown wrote: > 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 > Anyway, if you don't want to play nicely with the inotify mechanism of udev, you have to inject the "change" uevent manually for every device mdadm changes. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfjHrAAoJEANOs3ABTfJwgosP/i1btIR88rZ5z8eYB5P6sZqT XU0nUP1F4yMS1K4MOWDggQbuzcuxFowcnizg4Jje26c4z3kQ7Pj75GvqWwI3qqYp +TdG+idu7kGPeQtYa05I567pj20D6nWYxC78aGJPlBU6C0qPvA1yXb7ui4NPJcw4 2/oRH2BONpb62VCQKCB04rQhqOXnzp/9agaqAL7hJcUOsbJv8vceLW0rXD0RqTzO uQXQjUV2bYR73ySRZQWo2evaxZ/YgWDWL91h7R1O1wYvTclMNYqv+SQB9hDyrrDi mk4YFWxdRUmdIzyr6FkZUUTj1KSpmrW01PaaIi3ueHV27Pvmz+7+1jd5JVvw9GiM hm8Ob3baiGPMIfFZ7mfLBLlizBu0N4QTK5mm7D2btFS9phHb9/QzNpBtdAB15CTQ 8UF4IZg9HnbkG8XAedW97D3QS40873kPp7UPtsScnFe7+VcOh05s3AzF9zznji/C kEcpPIjthK50RLniWBEoKNEjbfdpyF1PLsvQ7GkQNIoUHSveCPJQmaG0GV8AIPht tNteLCImjeaUf57bi/BfKk5L42dwe8wqfmBGw40nbzmavRrYHEvUK3CMMR3C7TRZ lvGzXInwxJpR2hkN8A9nX13FOX3YtVLXUtl5R6CVvcrYM6ZK78D1z5liiZcfKv2Y XiUwQtkeIlscVHeW1QdO =fzOH -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 8:40 ` Harald Hoyer @ 2013-04-29 8:45 ` Harald Hoyer 2013-04-29 8:54 ` Harald Hoyer 0 siblings, 1 reply; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 8:45 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/29/2013 10:40 AM, Harald Hoyer wrote: > On 04/29/2013 08:53 AM, NeilBrown wrote: >> On Mon, 29 Apr 2013 08:32:31 +0200 Harald Hoyer <harald@redhat.com> >> wrote: >>> 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 > > > > Anyway, if you don't want to play nicely with the inotify mechanism of > udev, you have to inject the "change" uevent manually for every device > mdadm changes. > *rant mode on* I think we should drop the inotify mechanism from udev and say "f*** you" to the tools... If they change s.th. on the disk, they can also emit a "change" uevent to the kernel themselves. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfjM3AAoJEANOs3ABTfJwRwoP/jtGvFdNonRF71txWxTzvsas QErwTrojNdpRT850rCHIMNF9x/iYSMaVJcMGBx+H3zyZCDLiYwr7qvXUNWh9frC2 FQvJEqhpoH4ku17wT2eX4XYhIOC7/qudsS9EUfQ/NZw4her2uXbHosSJjfBjsp0f voVaU1dXZ4jsyA5nybH2fN0L70HpMei4AU9/lhI5UtBZa6erOe3o/kc0rT+E9o9z n4l+r6DI2zsgw/Cg0O59N6G9otd2zYuj2QLosPmG4Rx0yEcoC8Tn04ncJ8e5Qw4E GW0sm7SvP2tSlQvZFltKpGjEMPuKMXkcmgonkmGpZGSzpxF2evH+6SyK0lN4E7IK Guz6s5r+ecj6WWGneX7H+RD7EV4UIYHirr8txhS7QXmPiXR0+OJ4S5BPGdnxLBUf D/ttJUwA9sSSGdVx4M5iH39o/cC3I6XdUXqa7A6gpEsNZHE44masIVfdUIOqkv9a 1Ok3sArMWFkE+RtsRltzmcnGGyj58W4wBwtMp5gzbqfSGqpONzSt4uETzs1cTJWt qQdWMGFsb4WKjHmjoG0ohr70r5zNubpeXfQlWzAFcFSWcqPmKK4J/Wk9hrB2kBo/ TNdgAxyRJOzGh/uNF0XVbEuL2dfIxjc25AadYJs9eq3wUQrs5C3ppL3lHI4/vKFi AAwQBFAqj/b3MBJ5FR95 =r9Au -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] prevent double open(O_RDWR) on raid creation 2013-04-29 8:45 ` Harald Hoyer @ 2013-04-29 8:54 ` Harald Hoyer 0 siblings, 0 replies; 11+ messages in thread From: Harald Hoyer @ 2013-04-29 8:54 UTC (permalink / raw) To: NeilBrown; +Cc: Jes.Sorensen, linux-raid, Kay Sievers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/29/2013 10:45 AM, Harald Hoyer wrote: > On 04/29/2013 10:40 AM, Harald Hoyer wrote: >> On 04/29/2013 08:53 AM, NeilBrown wrote: >>> On Mon, 29 Apr 2013 08:32:31 +0200 Harald Hoyer <harald@redhat.com> >>> wrote: >>>> 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 > > > >> Anyway, if you don't want to play nicely with the inotify mechanism of >> udev, you have to inject the "change" uevent manually for every device >> mdadm changes. > > *rant mode on* > > I think we should drop the inotify mechanism from udev and say "f*** you" > to the tools... If they change s.th. on the disk, they can also emit a > "change" uevent to the kernel themselves. > Oh, and the tools then also have wait for udev to settle.. Imagine a script doing: # mdadm -A --uuid=<my-UUID> .... # mkfs -L mylabel /dev/disk/by-uuid/<my-MD-UUID> # mount /dev/disk/by-label/mylabel /mnt This is all broken by design today: - - there is no guaranteed change event - - there is no automatic udev settle -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRfjUtAAoJEANOs3ABTfJw/8IP/jX1RUgSQQwqM5QG1JUVXOZT Q/Uzuf4bDSdlHMrM0FjH6KNNDgh80fqwKBnb/sCsSNVRYgHdlLJXTdFYfzzWPk4t 2q7Rw/M8xQS9OnqzFy8BhkdvChyW8t9Pl2Owt4+6B++GJ+2ZZksb66ggsGca4bUE XjLDUVJ8zk+VjVyg0yFMtUMwbJ88q14I2b+eAnCenXt/RoIGqMIX5ds8PEiowQeo zVMjo0nXHyaDZPV6DkDIGtQ2vsnLGTc2V+SWzTH8vZJwbHERQNQczkcSiVFmt1JA kgOkcSiq80FSrNAkD/gYdmZ9Qy4Txo5y1/gnaoWubehkVctQpezdjWslnCY0HTKL ledREAUq562m9I5etIUINWjUyWJFLyBDD3a9TI3psg1dV4nNr7MmTf1OsCBaIhZt EvqXTyo03K0tgpQHUJCj/ZgNejgvoMJGVYIxMdIC64+gZcM3iHqMkkJjKHTvwc+7 Hh0Mn8L/XwYX5PoBrOJrg3XAAt4S97gA/Opv90q7VrEz47+04KHzI5T4PQ2B8MzA S+CZyDGMAazwSXTmaaEenXOaQKnYepiMQ/kFMM5teyaRKOPg3HqMPHWQnJq+/+bC ktA3X7wFQ5QLa8v5G2TpVPxvwTY+8gpmYWzXx+qhFZAMQne3RLd7zQiclNzUFkLK Nhnvkqel+fvi/0rFVx/X =SYBt -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-04-29 8:54 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).