[PATCH 0/2] aio: intialize kiocb private

[PATCH 0/2] aio: intialize kiocb private

[hubcap]

From: Mike Marshall <hubcap@omnibond.com>

"aio: don't zero entire aio_kiocb aio_get_req" triggered these
BUG_ONs in orangefs file.c. Obviously the BUG_ONs need to go.

There might be other filesystems that use iocb->private without
failing in a way that is as obvious as BUG_ON, though I didn't
see any with grep.

After discussion on fsdevel, Jens Axboe suggested I should send
in a patch that continues to set iocb->private to NULL just in case.

Also... phooey on fighting gmail when trying to send patches with
git-send-email... mail.kernel.org works great!

-Mike

[PATCH 1/2] orangefs: remove two un-needed BUG_ONs...

[hubcap]

From: Mike Marshall <hubcap@omnibond.com>

Signed-off-by: Mike Marshall <hubcap@omnibond.com>
---
 fs/orangefs/file.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index a5a2fe76568f..b094d3d79354 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -398,8 +398,6 @@ static ssize_t orangefs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter
 	loff_t pos = iocb->ki_pos;
 	ssize_t rc = 0;
 
-	BUG_ON(iocb->private);
-
 	gossip_debug(GOSSIP_FILE_DEBUG, "orangefs_file_read_iter\n");
 
 	orangefs_stats.reads++;
@@ -416,8 +414,6 @@ static ssize_t orangefs_file_write_iter(struct kiocb *iocb, struct iov_iter *ite
 	loff_t pos;
 	ssize_t rc;
 
-	BUG_ON(iocb->private);
-
 	gossip_debug(GOSSIP_FILE_DEBUG, "orangefs_file_write_iter\n");
 
 	inode_lock(file->f_mapping->host);
-- 
2.20.1

Re: [PATCH 1/2] orangefs: remove two un-needed BUG_ONs...

[Christoph Hellwig]

Didn't these catch the issue you fix in patch 2?

Re: [PATCH 1/2] orangefs: remove two un-needed BUG_ONs...

[Mike Marshall]

Yes, but I don't think they should be there. You put them into
pvfs2_file_aio_read_iovec way back when (Martin guesses you
were making sure that you had fully eradicated private's use)
and I left them in when I changed that function over to
pvfs2_file_read_iter. After that, they've just gone along for
the ride... BUG_ONs that the maintainer can't justify
might be the kind of thing that would make Linus violate
the new code of conduct :-) ...

-Mike

On Wed, Feb 6, 2019 at 1:58 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Didn't these catch the issue you fix in patch 2?

[PATCH 2/2] aio: initialize kiocb private in case any filesystems expect it.

[hubcap]

From: Mike Marshall <hubcap@omnibond.com>

A recent optimization had left private uninitialized.

Signed-off-by: Mike Marshall <hubcap@omnibond.com>
---
 fs/aio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/aio.c b/fs/aio.c
index b906ff70c90f..aaaaf4d12c73 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1436,6 +1436,7 @@ static int aio_prep_rw(struct kiocb *req, const struct iocb *iocb)
 	if (unlikely(!req->ki_filp))
 		return -EBADF;
 	req->ki_complete = aio_complete_rw;
+	req->private = NULL;
 	req->ki_pos = iocb->aio_offset;
 	req->ki_flags = iocb_flags(req->ki_filp);
 	if (iocb->aio_flags & IOCB_FLAG_RESFD)
-- 
2.20.1

Re: [PATCH 2/2] aio: initialize kiocb private in case any filesystems expect it.

[Christoph Hellwig]

On Tue, Feb 05, 2019 at 02:13:35PM -0500, hubcap@kernel.org wrote:
> From: Mike Marshall <hubcap@omnibond.com>
> 
> A recent optimization had left private uninitialized.
> 
> Signed-off-by: Mike Marshall <hubcap@omnibond.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 0/2] aio: intialize kiocb private

[Jens Axboe]

On 2/5/19 12:13 PM, hubcap@kernel.org wrote:
> From: Mike Marshall <hubcap@omnibond.com>
> 
> "aio: don't zero entire aio_kiocb aio_get_req" triggered these
> BUG_ONs in orangefs file.c. Obviously the BUG_ONs need to go.
> 
> There might be other filesystems that use iocb->private without
> failing in a way that is as obvious as BUG_ON, though I didn't
> see any with grep.
> 
> After discussion on fsdevel, Jens Axboe suggested I should send
> in a patch that continues to set iocb->private to NULL just in case.
> 
> Also... phooey on fighting gmail when trying to send patches with
> git-send-email... mail.kernel.org works great!

Applied 1/2, 2/2 should probably just go in separately with other
orangefs patches, when appropriate.

-- 
Jens Axboe