[v2,02/15] CIFS: Add support for direct pages in rdata

Message ID 20180530194807.31657-3-longli@linuxonhyperv.com
State New
Headers show
Series
  • CIFS: Add direct I/O support
Related show

Commit Message

Long Li May 30, 2018, 7:47 p.m.
From: Long Li <longli@microsoft.com>

Add a function to allocate rdata without allocating pages for data
transfer. This gives the caller an option to pass a number of pages
that point to the data buffer.

rdata is still reponsible for free those pages after it's done.

Signed-off-by: Long Li <longli@microsoft.com>
---
 fs/cifs/cifsglob.h |  2 +-
 fs/cifs/file.c     | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

Comments

Ruhl, Michael J May 30, 2018, 8:27 p.m. | #1
>-----Original Message-----
>From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>owner@vger.kernel.org] On Behalf Of Long Li
>Sent: Wednesday, May 30, 2018 3:48 PM
>To: Steve French <sfrench@samba.org>; linux-cifs@vger.kernel.org; samba-
>technical@lists.samba.org; linux-kernel@vger.kernel.org; linux-
>rdma@vger.kernel.org
>Cc: Long Li <longli@microsoft.com>
>Subject: [Patch v2 02/15] CIFS: Add support for direct pages in rdata
>
>From: Long Li <longli@microsoft.com>
>
>Add a function to allocate rdata without allocating pages for data
>transfer. This gives the caller an option to pass a number of pages
>that point to the data buffer.
>
>rdata is still reponsible for free those pages after it's done.
>
>Signed-off-by: Long Li <longli@microsoft.com>
>---
> fs/cifs/cifsglob.h |  2 +-
> fs/cifs/file.c     | 23 ++++++++++++++++++++---
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
>diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>index 8d16c3e..56864a87 100644
>--- a/fs/cifs/cifsglob.h
>+++ b/fs/cifs/cifsglob.h
>@@ -1179,7 +1179,7 @@ struct cifs_readdata {
> 	unsigned int			tailsz;
> 	unsigned int			credits;
> 	unsigned int			nr_pages;
>-	struct page			*pages[];
>+	struct page			**pages;
> };
>
> struct cifs_writedata;
>diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>index 23fd430..1c98293 100644
>--- a/fs/cifs/file.c
>+++ b/fs/cifs/file.c
>@@ -2880,13 +2880,13 @@ cifs_strict_writev(struct kiocb *iocb, struct
>iov_iter *from)
> }
>
> static struct cifs_readdata *
>-cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
>+cifs_readdata_direct_alloc(struct page **pages, work_func_t complete)
> {
> 	struct cifs_readdata *rdata;
>
>-	rdata = kzalloc(sizeof(*rdata) + (sizeof(struct page *) * nr_pages),
>-			GFP_KERNEL);
>+	rdata = kzalloc(sizeof(*rdata), GFP_KERNEL);
> 	if (rdata != NULL) {
>+		rdata->pages = pages;
> 		kref_init(&rdata->refcount);
> 		INIT_LIST_HEAD(&rdata->list);
> 		init_completion(&rdata->done);
>@@ -2896,6 +2896,22 @@ cifs_readdata_alloc(unsigned int nr_pages,
>work_func_t complete)
> 	return rdata;
> }
>
>+static struct cifs_readdata *
>+cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
>+{
>+	struct page **pages =
>+		kzalloc(sizeof(struct page *) * nr_pages, GFP_KERNEL);
>+	struct cifs_readdata *ret = NULL;
>+
>+	if (pages) {
>+		ret = cifs_readdata_direct_alloc(pages, complete);
>+		if (!ret)
>+			kfree(pages);
>+	}
>+
>+	return ret;
>+}
>+
> void
> cifs_readdata_release(struct kref *refcount)
> {
>@@ -2910,6 +2926,7 @@ cifs_readdata_release(struct kref *refcount)
> 	if (rdata->cfile)
> 		cifsFileInfo_put(rdata->cfile);
>
>+	kvfree(rdata->pages);

Is the kvfree() correct?

You use kzalloc() and kfree in cifs_readdata_alloc().

Mike

> 	kfree(rdata);
> }
>
>--
>2.7.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Long Li May 30, 2018, 8:57 p.m. | #2
> Subject: RE: [Patch v2 02/15] CIFS: Add support for direct pages in rdata
> 
> >-----Original Message-----
> >From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> >owner@vger.kernel.org] On Behalf Of Long Li
> >Sent: Wednesday, May 30, 2018 3:48 PM
> >To: Steve French <sfrench@samba.org>; linux-cifs@vger.kernel.org;
> >samba- technical@lists.samba.org; linux-kernel@vger.kernel.org; linux-
> >rdma@vger.kernel.org
> >Cc: Long Li <longli@microsoft.com>
> >Subject: [Patch v2 02/15] CIFS: Add support for direct pages in rdata
> >
> >From: Long Li <longli@microsoft.com>
> >
> >Add a function to allocate rdata without allocating pages for data
> >transfer. This gives the caller an option to pass a number of pages
> >that point to the data buffer.
> >
> >rdata is still reponsible for free those pages after it's done.
> >
> >Signed-off-by: Long Li <longli@microsoft.com>
> >---
> > fs/cifs/cifsglob.h |  2 +-
> > fs/cifs/file.c     | 23 ++++++++++++++++++++---
> > 2 files changed, 21 insertions(+), 4 deletions(-)
> >
> >diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
> >8d16c3e..56864a87 100644
> >--- a/fs/cifs/cifsglob.h
> >+++ b/fs/cifs/cifsglob.h
> >@@ -1179,7 +1179,7 @@ struct cifs_readdata {
> > 	unsigned int			tailsz;
> > 	unsigned int			credits;
> > 	unsigned int			nr_pages;
> >-	struct page			*pages[];
> >+	struct page			**pages;
> > };
> >
> > struct cifs_writedata;
> >diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 23fd430..1c98293
> >100644
> >--- a/fs/cifs/file.c
> >+++ b/fs/cifs/file.c
> >@@ -2880,13 +2880,13 @@ cifs_strict_writev(struct kiocb *iocb, struct
> >iov_iter *from)  }
> >
> > static struct cifs_readdata *
> >-cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
> >+cifs_readdata_direct_alloc(struct page **pages, work_func_t complete)
> > {
> > 	struct cifs_readdata *rdata;
> >
> >-	rdata = kzalloc(sizeof(*rdata) + (sizeof(struct page *) * nr_pages),
> >-			GFP_KERNEL);
> >+	rdata = kzalloc(sizeof(*rdata), GFP_KERNEL);
> > 	if (rdata != NULL) {
> >+		rdata->pages = pages;
> > 		kref_init(&rdata->refcount);
> > 		INIT_LIST_HEAD(&rdata->list);
> > 		init_completion(&rdata->done);
> >@@ -2896,6 +2896,22 @@ cifs_readdata_alloc(unsigned int nr_pages,
> >work_func_t complete)
> > 	return rdata;
> > }
> >
> >+static struct cifs_readdata *
> >+cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete) {
> >+	struct page **pages =
> >+		kzalloc(sizeof(struct page *) * nr_pages, GFP_KERNEL);
> >+	struct cifs_readdata *ret = NULL;
> >+
> >+	if (pages) {
> >+		ret = cifs_readdata_direct_alloc(pages, complete);
> >+		if (!ret)
> >+			kfree(pages);
> >+	}
> >+
> >+	return ret;
> >+}
> >+
> > void
> > cifs_readdata_release(struct kref *refcount)  { @@ -2910,6 +2926,7 @@
> >cifs_readdata_release(struct kref *refcount)
> > 	if (rdata->cfile)
> > 		cifsFileInfo_put(rdata->cfile);
> >
> >+	kvfree(rdata->pages);
> 
> Is the kvfree() correct?
> 
> You use kzalloc() and kfree in cifs_readdata_alloc().

This function is shared by both non-direct I/O and direct I/O code paths.

Direct I/O uses kvmalloc to allocate pages, so kvfree is used here to handle both cases.

> 
> Mike
> 
> > 	kfree(rdata);
> > }
> >
> >--
> >2.7.4
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> >in the body of a message to majordomo@vger.kernel.org More majordomo
> >info at
> >https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.k
> e
> >rnel.org%2Fmajordomo-
> info.html&data=02%7C01%7Clongli%40microsoft.com%7C
> >e810388a534643737ab108d5c66bd6df%7C72f988bf86f141af91ab2d7cd011db
> 47%7C1
> >%7C0%7C636633088833938755&sdata=iHKiji2rUhLHpbH5x13SJBWCvHExSr4a
> rz9Xiv3
> >1rMQ%3D&reserved=0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.ke
> rnel.org%2Fmajordomo-
> info.html&data=02%7C01%7Clongli%40microsoft.com%7Ce810388a53464373
> 7ab108d5c66bd6df%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63
> 6633088833938755&sdata=iHKiji2rUhLHpbH5x13SJBWCvHExSr4arz9Xiv31rMQ
> %3D&reserved=0
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8d16c3e..56864a87 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1179,7 +1179,7 @@  struct cifs_readdata {
 	unsigned int			tailsz;
 	unsigned int			credits;
 	unsigned int			nr_pages;
-	struct page			*pages[];
+	struct page			**pages;
 };
 
 struct cifs_writedata;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 23fd430..1c98293 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2880,13 +2880,13 @@  cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from)
 }
 
 static struct cifs_readdata *
-cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
+cifs_readdata_direct_alloc(struct page **pages, work_func_t complete)
 {
 	struct cifs_readdata *rdata;
 
-	rdata = kzalloc(sizeof(*rdata) + (sizeof(struct page *) * nr_pages),
-			GFP_KERNEL);
+	rdata = kzalloc(sizeof(*rdata), GFP_KERNEL);
 	if (rdata != NULL) {
+		rdata->pages = pages;
 		kref_init(&rdata->refcount);
 		INIT_LIST_HEAD(&rdata->list);
 		init_completion(&rdata->done);
@@ -2896,6 +2896,22 @@  cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
 	return rdata;
 }
 
+static struct cifs_readdata *
+cifs_readdata_alloc(unsigned int nr_pages, work_func_t complete)
+{
+	struct page **pages =
+		kzalloc(sizeof(struct page *) * nr_pages, GFP_KERNEL);
+	struct cifs_readdata *ret = NULL;
+
+	if (pages) {
+		ret = cifs_readdata_direct_alloc(pages, complete);
+		if (!ret)
+			kfree(pages);
+	}
+
+	return ret;
+}
+
 void
 cifs_readdata_release(struct kref *refcount)
 {
@@ -2910,6 +2926,7 @@  cifs_readdata_release(struct kref *refcount)
 	if (rdata->cfile)
 		cifsFileInfo_put(rdata->cfile);
 
+	kvfree(rdata->pages);
 	kfree(rdata);
 }