From mboxrd@z Thu Jan 1 00:00:00 1970 From: NeilBrown Subject: Re: [MDADM PATCH 2/2] Give enough time to udev to handle events Date: Tue, 19 Sep 2017 08:49:19 +0200 Message-ID: <87r2v3qj2o.fsf@notabene.neil.brown.name> References: <1505801207-10096-1-git-send-email-xni@redhat.com> <1505801207-10096-3-git-send-email-xni@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: In-Reply-To: <1505801207-10096-3-git-send-email-xni@redhat.com> Sender: linux-raid-owner@vger.kernel.org To: Xiao Ni , linux-raid@vger.kernel.org Cc: jes.sorensen@gmail.com List-Id: linux-raid.ids --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Tue, Sep 19 2017, Xiao Ni wrote: > After mdadm -S /dev/md0, the device node /dev/md0 still exists. The Remove > events are generated by md_free() -> del_gendisk() -> blk_unregister_queu= e(). > After calling close(mdfd) the Remove events is generated. We should give = udev > a little time to handle the event. > > I tried usleep(100*1000), but the problem still can be reproduced sometim= e. > So I choose to sleep(1). Because after close(mdfd) it can be generated CH= ANGE > events too. So it's ok to choose to sleep(1) to wait udev to handle CHANGE > events. I really don't like this approach. The fact that 1 second works for you is no guarantee that it will work for everybody. We have a few sleeps in the code already, but I don't like them either. Let's try not to add more. If there is some event that you want to wait for, wait for that event. e.g. if you want to wait for /dev/md0 to disappear then write a loop: while /dev/md0 exists usleep(1000) But I'm still not convinced that this is really needed. If it is, then maybe some sort of kernel fix would be better. NeilBrown > > Signed-off-by: Xiao Ni > --- > Create.c | 2 +- > mdadm.c | 16 +++++++++++++++- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/Create.c b/Create.c > index 239545f..24e852e 100644 > --- a/Create.c > +++ b/Create.c > @@ -1054,7 +1054,7 @@ int Create(struct supertype *st, char *mddev, > /* Give udev a moment to process the Change event caused > * by the close. > */ > - usleep(100*1000); > + sleep(1); > udev_unblock(); > return 0; >=20=20 > diff --git a/mdadm.c b/mdadm.c > index 7cdcdba..2905dea 100644 > --- a/mdadm.c > +++ b/mdadm.c > @@ -1734,8 +1734,14 @@ int main(int argc, char *argv[]) > autodetect(); > break; > } > - if (mdfd > 0) > + > + if (mdfd > 0) { > close(mdfd); > + /* Give udev a moment to process the udev event caused > + * by the close. > + */ > + sleep(1); > + } > exit(rv); > } >=20=20 > @@ -1897,6 +1903,10 @@ static int stop_scan(int verbose) > else > progress =3D 1; > close(mdfd); > + /* Give udev a moment to process the Remove event caused > + * by the close. > + */ > + sleep(1); > } >=20=20 > put_md_name(name); > @@ -1997,6 +2007,10 @@ static int misc_list(struct mddev_dev *devlist, > break; > } > close(mdfd); > + /* Give udev a moment to process the udev event caused > + * by the close. > + */ > + sleep(1); > } else > rv |=3D 1; > } > --=20 > 2.7.4 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAlnAve8ACgkQOeye3VZi gbnwKQ/7B8JftLcBKABZw+yhyuxyC3ZboA4SHZhzF3GE8hSapNSml8OynZeAVqKQ 1KvmRT6eid4AVby4H4i9jtrEsKWZ9O5fBSu/OFO+7UoFLJKF02+eU/kiDy8xFuME 6qZ5K/AIWQXcF0BeT2P7+s9ceBQ1rxPQUa0H4awude8FblPXE6JEIxxV6bQibq2N avxALScKgQlDVcqNtKhQZGEM3NcJBIpidKwP5EJdCnrK5ASM/nWI7/1rNK8Adecq wIH7zl5aVjncfdbCenIg/Tdq/6OzUts6izuKjWG8uGRJCFp3epeNgPc1r9hi2wsH vWJUJjcqluZtW2wUCD+/fLtsDGPC76aw0DsT2G5sps58Xh7ujfbFbcK5iS8Zcipb ZedJU+vC3RFvkLv1p/Yf4QQ10G+YRZn7eEastqSrNA7FkMhLQ5lZWTxNrf7YzQ9G A+CpNPahay50ckJPIOx3FAJsGU4LSBwW5QD64h1rgv0IfB/uEBhJRaJ1VqT+BnI0 /gAj46TtoOvVQbMiADswOpZ76+kg3X4HuTyRAIE+Rd7kGeIXtGhI2DWYDl0YI/8d mxSwtivjoXYiduSDZwGgfmXTAU+wZOI+rVUkuLqrdKcE0wsatLpL7uHTFDuJmfgG zgrwyw+SSO1tytLZEjxO2NuurW2IKotSMbqkXkT4fA0/BymT2x4= =O6gz -----END PGP SIGNATURE----- --=-=-=--