xfs
[Top] [All Lists]

Re: [PATCH] xfs: don't allocate an ioend for direct I/O completions

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: don't allocate an ioend for direct I/O completions
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 2 Feb 2015 10:04:03 +1100
Cc: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150130144223.GA27441@xxxxxxxxxxxxxx>
References: <1422485661-520-1-git-send-email-hch@xxxxxx> <20150130144223.GA27441@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Jan 30, 2015 at 09:42:23AM -0500, Brian Foster wrote:
> On Wed, Jan 28, 2015 at 11:54:21PM +0100, Christoph Hellwig wrote:
> > Back in the days when the direct I/O ->end_io callback could be called
> > from interrupt context for AIO we needed a structure to hand off to the
> > workqueue, and reused the ioend structure for this purpose.  These days
> > ->end_io is always called from user or workqueue context, which allows us
> > to avoid this memory allocation and simplify the code significantly.
> > 
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> 
> Looks mostly Ok to me. In fact, with xfs_finish_ioend_sync() calling
> xfs_end_io() directly, I don't see how we currently get into the wq at
> all. Anyways, a few notes...

I've pulled this in after making the couple of minor changes that
Brian suggested....

> > @@ -1507,39 +1514,17 @@ xfs_vm_direct_IO(
> >  {
> >     struct inode            *inode = iocb->ki_filp->f_mapping->host;
> >     struct block_device     *bdev = xfs_find_bdev_for_inode(inode);
> > -   struct xfs_ioend        *ioend = NULL;
> > -   ssize_t                 ret;
> >  
> >     if (rw & WRITE) {
> 
> A nit, but I guess you could kill the braces here now too.

Given it's a multi-line return statement, the braces are fine. FWIW,
when we have a if () { return ...} else { return ... } we normally
kill the else. i.e:

        if (rw & WRITE) {
                return foo(
                        bar,
                        baz);
        }
        return .....;

So I modified it like this.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>