* [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications @ 2014-05-30 13:18 Artur Paszkiewicz 2014-06-02 2:36 ` NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: Artur Paszkiewicz @ 2014-05-30 13:18 UTC (permalink / raw) To: neilb; +Cc: linux-raid, pawel.baldysiak, Artur Paszkiewicz If the checksum verification fails in mdadm and mdmon is running, retry the load to get a consistent snapshot of the mpb. Based on db575f3b Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> --- super-intel.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/super-intel.c b/super-intel.c index f0a7ab5..037c018 100644 --- a/super-intel.c +++ b/super-intel.c @@ -4422,6 +4422,7 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) { struct intel_super *super; int rv; + int retry; if (test_partition(fd)) /* IMSM not allowed on partitions */ @@ -4444,6 +4445,22 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) } rv = load_and_parse_mpb(fd, super, devname, 0); + /* retry the load if we might have raced against mdmon */ + if (rv == 3) { + struct mdstat_ent *mdstat = mdstat_by_component(fd2devnm(fd)); + + if (mdmon_running(mdstat->devnm) && getpid() != mdmon_pid(mdstat->devnm)) { + for (retry = 0; retry < 3; retry++) { + usleep(3000); + rv = load_and_parse_mpb(fd, super, devname, 0); + if (rv != 3) + break; + } + } + + free_mdstat(mdstat); + } + if (rv) { if (devname) pr_err("Failed to load all information " -- 1.8.4.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications 2014-05-30 13:18 [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications Artur Paszkiewicz @ 2014-06-02 2:36 ` NeilBrown 2014-06-02 13:02 ` Artur Paszkiewicz 0 siblings, 1 reply; 4+ messages in thread From: NeilBrown @ 2014-06-02 2:36 UTC (permalink / raw) To: Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak [-- Attachment #1: Type: text/plain, Size: 1776 bytes --] On Fri, 30 May 2014 15:18:33 +0200 Artur Paszkiewicz <artur.paszkiewicz@intel.com> wrote: > If the checksum verification fails in mdadm and mdmon is running, retry > the load to get a consistent snapshot of the mpb. > > Based on db575f3b > > Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> > Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> > --- > super-intel.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/super-intel.c b/super-intel.c > index f0a7ab5..037c018 100644 > --- a/super-intel.c > +++ b/super-intel.c > @@ -4422,6 +4422,7 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) > { > struct intel_super *super; > int rv; > + int retry; > > if (test_partition(fd)) > /* IMSM not allowed on partitions */ > @@ -4444,6 +4445,22 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) > } > rv = load_and_parse_mpb(fd, super, devname, 0); > > + /* retry the load if we might have raced against mdmon */ > + if (rv == 3) { > + struct mdstat_ent *mdstat = mdstat_by_component(fd2devnm(fd)); > + > + if (mdmon_running(mdstat->devnm) && getpid() != mdmon_pid(mdstat->devnm)) { > + for (retry = 0; retry < 3; retry++) { > + usleep(3000); > + rv = load_and_parse_mpb(fd, super, devname, 0); > + if (rv != 3) > + break; > + } > + } The only thing you use from mdstat is devnm, and that is the thing you passed to mdstat_by_component to get mdstat.... Can you just do char *devnm = fd2devnm(fd); if (mdmon_running(devnm) && ......) ?? NeilBrown > + > + free_mdstat(mdstat); > + } > + > if (rv) { > if (devname) > pr_err("Failed to load all information " [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications 2014-06-02 2:36 ` NeilBrown @ 2014-06-02 13:02 ` Artur Paszkiewicz 2014-06-02 23:10 ` NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: Artur Paszkiewicz @ 2014-06-02 13:02 UTC (permalink / raw) To: NeilBrown; +Cc: linux-raid, Baldysiak, Pawel On 06/02/2014 04:36 AM, NeilBrown wrote: > On Fri, 30 May 2014 15:18:33 +0200 Artur Paszkiewicz > <artur.paszkiewicz@intel.com> wrote: > >> If the checksum verification fails in mdadm and mdmon is running, retry >> the load to get a consistent snapshot of the mpb. >> >> Based on db575f3b >> >> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> >> Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> >> --- >> super-intel.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/super-intel.c b/super-intel.c >> index f0a7ab5..037c018 100644 >> --- a/super-intel.c >> +++ b/super-intel.c >> @@ -4422,6 +4422,7 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) >> { >> struct intel_super *super; >> int rv; >> + int retry; >> >> if (test_partition(fd)) >> /* IMSM not allowed on partitions */ >> @@ -4444,6 +4445,22 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) >> } >> rv = load_and_parse_mpb(fd, super, devname, 0); >> >> + /* retry the load if we might have raced against mdmon */ >> + if (rv == 3) { >> + struct mdstat_ent *mdstat = mdstat_by_component(fd2devnm(fd)); >> + >> + if (mdmon_running(mdstat->devnm) && getpid() != mdmon_pid(mdstat->devnm)) { >> + for (retry = 0; retry < 3; retry++) { >> + usleep(3000); >> + rv = load_and_parse_mpb(fd, super, devname, 0); >> + if (rv != 3) >> + break; >> + } >> + } > > The only thing you use from mdstat is devnm, and that is the thing you passed > to mdstat_by_component to get mdstat.... > > Can you just do > char *devnm = fd2devnm(fd); > if (mdmon_running(devnm) && ......) > > ?? > I can't do that because mdmon_running and mdmon_pid need a devnm of a container device, and the only thing we have here is the file descriptor of a component device. So I used mdstat_by_component to get the container devnm. Do you have an idea how to get that reliably without reading mdstat? I have overlooked that mdstat_by_component can return NULL here. I've added a check for this in the patch below. Thanks, Artur From dfb12870a482654b405ec1d4d9d3a8ba69a6290c Mon Sep 17 00:00:00 2001 From: Artur Paszkiewicz <artur.paszkiewicz@intel.com> Date: Tue, 27 May 2014 15:30:54 +0200 Subject: [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications If the checksum verification fails in mdadm and mdmon is running, retry the load to get a consistent snapshot of the mpb. Based on db575f3b Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> --- super-intel.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/super-intel.c b/super-intel.c index f0a7ab5..9dd807a 100644 --- a/super-intel.c +++ b/super-intel.c @@ -4422,6 +4422,7 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) { struct intel_super *super; int rv; + int retry; if (test_partition(fd)) /* IMSM not allowed on partitions */ @@ -4444,6 +4445,22 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) } rv = load_and_parse_mpb(fd, super, devname, 0); + /* retry the load if we might have raced against mdmon */ + if (rv == 3) { + struct mdstat_ent *mdstat = mdstat_by_component(fd2devnm(fd)); + + if (mdstat && mdmon_running(mdstat->devnm) && getpid() != mdmon_pid(mdstat->devnm)) { + for (retry = 0; retry < 3; retry++) { + usleep(3000); + rv = load_and_parse_mpb(fd, super, devname, 0); + if (rv != 3) + break; + } + } + + free_mdstat(mdstat); + } + if (rv) { if (devname) pr_err("Failed to load all information " ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications 2014-06-02 13:02 ` Artur Paszkiewicz @ 2014-06-02 23:10 ` NeilBrown 0 siblings, 0 replies; 4+ messages in thread From: NeilBrown @ 2014-06-02 23:10 UTC (permalink / raw) To: Artur Paszkiewicz; +Cc: linux-raid, Baldysiak, Pawel [-- Attachment #1: Type: text/plain, Size: 2472 bytes --] On Mon, 02 Jun 2014 15:02:59 +0200 Artur Paszkiewicz <artur.paszkiewicz@intel.com> wrote: > On 06/02/2014 04:36 AM, NeilBrown wrote: > > On Fri, 30 May 2014 15:18:33 +0200 Artur Paszkiewicz > > <artur.paszkiewicz@intel.com> wrote: > > > >> If the checksum verification fails in mdadm and mdmon is running, retry > >> the load to get a consistent snapshot of the mpb. > >> > >> Based on db575f3b > >> > >> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> > >> Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> > >> --- > >> super-intel.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/super-intel.c b/super-intel.c > >> index f0a7ab5..037c018 100644 > >> --- a/super-intel.c > >> +++ b/super-intel.c > >> @@ -4422,6 +4422,7 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) > >> { > >> struct intel_super *super; > >> int rv; > >> + int retry; > >> > >> if (test_partition(fd)) > >> /* IMSM not allowed on partitions */ > >> @@ -4444,6 +4445,22 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname) > >> } > >> rv = load_and_parse_mpb(fd, super, devname, 0); > >> > >> + /* retry the load if we might have raced against mdmon */ > >> + if (rv == 3) { > >> + struct mdstat_ent *mdstat = mdstat_by_component(fd2devnm(fd)); > >> + > >> + if (mdmon_running(mdstat->devnm) && getpid() != mdmon_pid(mdstat->devnm)) { > >> + for (retry = 0; retry < 3; retry++) { > >> + usleep(3000); > >> + rv = load_and_parse_mpb(fd, super, devname, 0); > >> + if (rv != 3) > >> + break; > >> + } > >> + } > > > > The only thing you use from mdstat is devnm, and that is the thing you passed > > to mdstat_by_component to get mdstat.... > > > > Can you just do > > char *devnm = fd2devnm(fd); > > if (mdmon_running(devnm) && ......) > > > > ?? > > > I can't do that because mdmon_running and mdmon_pid need a devnm of a > container device, and the only thing we have here is the file descriptor > of a component device. So I used mdstat_by_component to get the > container devnm. Do you have an idea how to get that reliably without > reading mdstat? > > I have overlooked that mdstat_by_component can return NULL here. I've > added a check for this in the patch below. Right, of course, yes. I've applied your patch - and thanks for the updated version. NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-02 23:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-30 13:18 [PATCH] imsm: retry load_and_parse_mpb if we suspect mdmon has made modifications Artur Paszkiewicz 2014-06-02 2:36 ` NeilBrown 2014-06-02 13:02 ` Artur Paszkiewicz 2014-06-02 23:10 ` NeilBrown
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.