From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jes Sorensen Subject: Re: [MDADM PATCH 2/2] Give enough time to udev to handle events Date: Mon, 2 Oct 2017 13:28:08 -0400 Message-ID: References: <1505801207-10096-1-git-send-email-xni@redhat.com> <1505801207-10096-3-git-send-email-xni@redhat.com> <87r2v3qj2o.fsf@notabene.neil.brown.name> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <87r2v3qj2o.fsf@notabene.neil.brown.name> Content-Language: en-US Sender: linux-raid-owner@vger.kernel.org To: NeilBrown , Xiao Ni , linux-raid@vger.kernel.org List-Id: linux-raid.ids On 09/19/2017 02:49 AM, NeilBrown wrote: > 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_queue(). >> 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 sometime. >> So I choose to sleep(1). Because after close(mdfd) it can be generated CHANGE >> 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. I agree completely - any case where we need a sleep() is a warning that there is probably a bigger problem that needs to be addressed. Cheers, Jes