Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 11 May 2023 20:03:12 +0900
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     Dan Carpenter <dan.carpenter@linaro.org>
Cc:     oe-kbuild@lists.linux.dev,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>,
        Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>,
        Stefan Roesch <shr@fb.com>, lkp@intel.com,
        oe-kbuild-all@lists.linux.dev, Clay Harris <bugs@claycon.org>,
        Dave Chinner <david@fromorbit.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        io-uring@vger.kernel.org
Subject: Re: [PATCH v2 4/6] kernfs: implement readdir FMODE_NOWAIT
Message-ID: <ZFzLcLHvzwqM-uek@codewreck.org>
References: <20230422-uring-getdents-v2-4-2db1e37dc55e@codewreck.org>
 <4e88ec58-be22-4b0c-968d-fa9a52764c98@kili.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <4e88ec58-be22-4b0c-968d-fa9a52764c98@kili.mountain>
Precedence: bulk

Dan Carpenter wrote on Thu, May 11, 2023 at 01:55:57PM +0300:
> fd7b9f7b9776b1 Tejun Heo          2013-11-28  1850  		if (!dir_emit(ctx, name, len, ino, type))
> fd7b9f7b9776b1 Tejun Heo          2013-11-28  1851  			return 0;
> 393c3714081a53 Minchan Kim        2021-11-18  1852  		down_read(&root->kernfs_rwsem);
>                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Needs to be deleted.

Uh, yes, sorry; I'm not sure how I let that slip, I guess I didn't hit
any dead lock because nothing ever tried to take a write lock after
getdents...
Thanks!

I expect there'll be other comments (this might not make it at all), so
I'll keep the v3 of this patch with this fix locally and resend after
other comments.

> a551138c4b3b9f Dominique Martinet 2023-05-10  1853  		if (ctx->flags & DIR_CONTEXT_F_NOWAIT) {
> a551138c4b3b9f Dominique Martinet 2023-05-10  1854  			if (!down_read_trylock(&root->kernfs_rwsem))
> a551138c4b3b9f Dominique Martinet 2023-05-10  1855  				return 0;
> 
> It's a bit strange the this doesn't return -EAGAIN;

That is on purpose: the getdents did work (dir_emit returned success at
least once), so the caller can process whatever was filled in the buffer
before calling iterate_shared() again.

If we were to return -EAGAIN here, we'd actually be throwing out the
entries we just filled in, and that's not what we want.

-- 
Dominique Martinet | Asmadeus