Re: [PATCH 2/5] cifs: add cifs_async_readv

[Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

2011/10/11 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Tue, 11 Oct 2011 13:30:14 +0400
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> 2011/9/6 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>> > ...which will allow cifs to do an asynchronous read call to the server.
>> > The caller will allocate and set up cifs_readdata for each READ_AND_X
>> > call that should be issued on the wire. The pages passed in are added
>> > to the pagecache, but not placed on the LRU list yet (as we need the
>> > page->lru to keep the pages on the list in the readdata).
>> >
>> > When cifsd identifies the mid, it will see that there is a special
>> > receive handler for the call, and use that to receive the rest of the
>> > frame. cifs_readv_receive will then marshal up a kvec array with
>> > kmapped pages from the pagecache, which eliminates one copy of the
>> > data. Once the data is received, the pages are added to the LRU list,
>> > set uptodate, and unlocked.
>> >
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  fs/cifs/cifsproto.h |   24 ++++
>> >  fs/cifs/cifssmb.c   |  356 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  fs/cifs/connect.c   |   26 ++--
>> >  3 files changed, 393 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> > index 51c0ebc..38406e5 100644
>> > --- a/fs/cifs/cifsproto.h
>> > +++ b/fs/cifs/cifsproto.h
>> > @@ -152,6 +152,12 @@ extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
>> >  extern int set_cifs_acl(struct cifs_ntsd *, __u32, struct inode *,
>> >                                const char *);
>> >
>> > +extern void dequeue_mid(struct mid_q_entry *mid, bool malformed);
>> > +extern int cifs_read_from_socket(struct TCP_Server_Info *server, char *buf,
>> > +                    unsigned int to_read);
>> > +extern int cifs_readv_from_socket(struct TCP_Server_Info *server,
>> > +               struct kvec *iov_orig, unsigned int nr_segs,
>> > +               unsigned int to_read);
>> >  extern void cifs_setup_cifs_sb(struct smb_vol *pvolume_info,
>> >                               struct cifs_sb_info *cifs_sb);
>> >  extern int cifs_match_super(struct super_block *, void *);
>> > @@ -441,6 +447,24 @@ extern int E_md4hash(const unsigned char *passwd, unsigned char *p16);
>> >  extern int SMBencrypt(unsigned char *passwd, const unsigned char *c8,
>> >                        unsigned char *p24);
>> >
>> > +/* asynchronous read support */
>> > +struct cifs_readdata {
>> > +       struct cifsFileInfo             *cfile;
>> > +       struct address_space            *mapping;
>> > +       __u64                           offset;
>> > +       unsigned int                    bytes;
>> > +       pid_t                           pid;
>> > +       int                             result;
>> > +       struct list_head                pages;
>> > +       struct work_struct              work;
>> > +       unsigned int                    nr_iov;
>> > +       struct kvec                     iov[1];
>> > +};
>> > +
>> > +struct cifs_readdata *cifs_readdata_alloc(unsigned int nr_pages);
>> > +void cifs_readdata_free(struct cifs_readdata *rdata);
>> > +int cifs_async_readv(struct cifs_readdata *rdata);
>> > +
>> >  /* asynchronous write support */
>> >  struct cifs_writedata {
>> >        struct kref                     refcount;
>> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>> > index ae1ce01..ac72f28 100644
>> > --- a/fs/cifs/cifssmb.c
>> > +++ b/fs/cifs/cifssmb.c
>> > @@ -33,6 +33,8 @@
>> >  #include <linux/slab.h>
>> >  #include <linux/posix_acl_xattr.h>
>> >  #include <linux/pagemap.h>
>> > +#include <linux/swap.h>
>> > +#include <linux/task_io_accounting_ops.h>
>> >  #include <asm/uaccess.h>
>> >  #include "cifspdu.h"
>> >  #include "cifsglob.h"
>> > @@ -40,6 +42,7 @@
>> >  #include "cifsproto.h"
>> >  #include "cifs_unicode.h"
>> >  #include "cifs_debug.h"
>> > +#include "fscache.h"
>> >
>> >  #ifdef CONFIG_CIFS_POSIX
>> >  static struct {
>> > @@ -83,6 +86,9 @@ static struct {
>> >  #endif /* CONFIG_CIFS_WEAK_PW_HASH */
>> >  #endif /* CIFS_POSIX */
>> >
>> > +/* Forward declarations */
>> > +static void cifs_readv_complete(struct work_struct *work);
>> > +
>> >  /* Mark as invalid, all open files on tree connections since they
>> >    were closed when session to server was lost */
>> >  static void mark_open_files_invalid(struct cifs_tcon *pTcon)
>> > @@ -1375,6 +1381,356 @@ openRetry:
>> >        return rc;
>> >  }
>> >
>> > +struct cifs_readdata *
>> > +cifs_readdata_alloc(unsigned int nr_pages)
>> > +{
>> > +       struct cifs_readdata *rdata;
>> > +
>> > +       /* readdata + 1 kvec for each page */
>> > +       rdata = kzalloc(sizeof(*rdata) +
>> > +                       sizeof(struct kvec) * nr_pages, GFP_KERNEL);
>> > +       if (rdata != NULL) {
>> > +               INIT_WORK(&rdata->work, cifs_readv_complete);
>> > +               INIT_LIST_HEAD(&rdata->pages);
>> > +       }
>> > +       return rdata;
>> > +}
>> > +
>> > +void
>> > +cifs_readdata_free(struct cifs_readdata *rdata)
>> > +{
>> > +       cifsFileInfo_put(rdata->cfile);
>> > +       kfree(rdata);
>> > +}
>> > +
>> > +/*
>> > + * Discard any remaining data in the current SMB. To do this, we borrow the
>> > + * current bigbuf.
>> > + */
>> > +static int
>> > +cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>> > +{
>> > +       READ_RSP *rsp = (READ_RSP *)server->smallbuf;
>> > +       unsigned int rfclen = be32_to_cpu(rsp->hdr.smb_buf_length);
>> > +       int remaining = rfclen + 4 - server->total_read;
>> > +       struct cifs_readdata *rdata = mid->callback_data;
>> > +
>> > +       while (remaining > 0) {
>> > +               int length;
>> > +
>> > +               length = cifs_read_from_socket(server, server->bigbuf,
>> > +                               min_t(unsigned int, remaining,
>> > +                                       CIFSMaxBufSize + MAX_CIFS_HDR_SIZE));
>> > +               if (length < 0)
>> > +                       return length;
>> > +               server->total_read += length;
>> > +               remaining -= length;
>> > +       }
>> > +
>> > +       dequeue_mid(mid, rdata->result);
>> > +       return 0;
>> > +}
>> > +
>> > +static int
>> > +cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>> > +{
>> > +       int length, len;
>> > +       unsigned int data_offset, remaining, data_len;
>> > +       struct cifs_readdata *rdata = mid->callback_data;
>> > +       READ_RSP *rsp = (READ_RSP *)server->smallbuf;
>> > +       unsigned int rfclen = be32_to_cpu(rsp->hdr.smb_buf_length) + 4;
>> > +       u64 eof;
>> > +       pgoff_t eof_index;
>> > +       struct page *page, *tpage;
>> > +
>> > +       cFYI(1, "%s: mid=%u offset=%llu bytes=%u", __func__,
>> > +               mid->mid, rdata->offset, rdata->bytes);
>> > +
>> > +       /*
>> > +        * read the rest of READ_RSP header (sans Data array), or whatever we
>> > +        * can if there's not enough data. At this point, we've read down to
>> > +        * the Mid.
>> > +        */
>> > +       len = min_t(unsigned int, rfclen, sizeof(*rsp)) -
>> > +                       sizeof(struct smb_hdr) + 1;
>> > +
>> > +       rdata->iov[0].iov_base = server->smallbuf + sizeof(struct smb_hdr) - 1;
>> > +       rdata->iov[0].iov_len = len;
>> > +
>> > +       length = cifs_readv_from_socket(server, rdata->iov, 1, len);
>> > +       if (length < 0)
>> > +               return length;
>> > +       server->total_read += length;
>> > +
>> > +       /* Was the SMB read successful? */
>> > +       rdata->result = map_smb_to_linux_error(&rsp->hdr, false);
>> > +       if (rdata->result != 0) {
>> > +               cFYI(1, "%s: server returned error %d", __func__,
>> > +                       rdata->result);
>> > +               return cifs_readv_discard(server, mid);
>> > +       }
>> > +
>> > +       /* Is there enough to get to the rest of the READ_RSP header? */
>> > +       if (server->total_read < sizeof(READ_RSP)) {
>> > +               cFYI(1, "%s: server returned short header. got=%u expected=%lu",
>> > +                       __func__, server->total_read, sizeof(READ_RSP));
>>
>> sizeof should be casted to unsigned long to prevent compiler warnings.
>>
>
> Actually...I think what I need to do here and in the other places you
> pointed out is to use "%z". I'll fix and respin.
>
> Thanks for the review...
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>

No problem. Yes, %z is ok too.

-- 
Best regards,
Pavel Shilovsky.