| Message ID | 20180530194807.31657-3-longli@linuxonhyperv.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>-----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
> 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
On 5/30/2018 3:47 PM, Long Li wrote: > 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. "Caller" is still responsible? Or is the rdata somehow freeing itself via another mechanism? > > 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; Technically speaking, these are syntactically equivalent. It may not be worth changing this historic definition. Tom. > }; > > 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); > } > > -- 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
PiBGcm9tOiBUb20gVGFscGV5IDx0b21AdGFscGV5LmNvbT4NCj4gU2VudDogU2F0dXJkYXksIEp1 bmUgMjMsIDIwMTggNjo1MCBQTQ0KPiBUbzogTG9uZyBMaSA8bG9uZ2xpQG1pY3Jvc29mdC5jb20+ OyBTdGV2ZSBGcmVuY2ggPHNmcmVuY2hAc2FtYmEub3JnPjsNCj4gbGludXgtY2lmc0B2Z2VyLmtl cm5lbC5vcmc7IHNhbWJhLXRlY2huaWNhbEBsaXN0cy5zYW1iYS5vcmc7IGxpbnV4LQ0KPiBrZXJu ZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eC1yZG1hQHZnZXIua2VybmVsLm9yZw0KPiBTdWJqZWN0 OiBSZTogW1BhdGNoIHYyIDAyLzE1XSBDSUZTOiBBZGQgc3VwcG9ydCBmb3IgZGlyZWN0IHBhZ2Vz IGluIHJkYXRhDQo+IA0KPiBPbiA1LzMwLzIwMTggMzo0NyBQTSwgTG9uZyBMaSB3cm90ZToNCj4g PiBGcm9tOiBMb25nIExpIDxsb25nbGlAbWljcm9zb2Z0LmNvbT4NCj4gPg0KPiA+IEFkZCBhIGZ1 bmN0aW9uIHRvIGFsbG9jYXRlIHJkYXRhIHdpdGhvdXQgYWxsb2NhdGluZyBwYWdlcyBmb3IgZGF0 YQ0KPiA+IHRyYW5zZmVyLiBUaGlzIGdpdmVzIHRoZSBjYWxsZXIgYW4gb3B0aW9uIHRvIHBhc3Mg YSBudW1iZXIgb2YgcGFnZXMNCj4gPiB0aGF0IHBvaW50IHRvIHRoZSBkYXRhIGJ1ZmZlci4NCj4g Pg0KPiA+IHJkYXRhIGlzIHN0aWxsIHJlcG9uc2libGUgZm9yIGZyZWUgdGhvc2UgcGFnZXMgYWZ0 ZXIgaXQncyBkb25lLg0KPiANCj4gIkNhbGxlciIgaXMgc3RpbGwgcmVzcG9uc2libGU/IE9yIGlz IHRoZSByZGF0YSBzb21laG93IGZyZWVpbmcgaXRzZWxmIHZpYSBhbm90aGVyDQo+IG1lY2hhbmlz bT8NCg0KSSBtZWFudCB0byBzYXkgZnJlZSB0aGUgYXJyYXkgInN0cnVjdCBwYWdlICoqcGFnZXMi LCBub3QgdGhlIHBhZ2VzIHRoZW1zZWx2ZXMuDQoNCkJlZm9yZSB0aGUgcGF0Y2gsIHRoZSBwYWdl IGFycmF5IGlzIGFsbG9jYXRlZCBhdCB0aGUgZW5kIG9mIHRoZSBzdHJ1Y3QgY2lmc19yZWFkZGF0 YToNCg0KICAgICAgcmRhdGEgPSBremFsbG9jKHNpemVvZigqcmRhdGEpICsgKHNpemVvZihzdHJ1 Y3QgcGFnZSAqKSAqIG5yX3BhZ2VzKSwNCiAgICAgICAgICAgICAgICAgICAgICAgR0ZQX0tFUk5F TCk7DQoNCklmIHRoaXMgaXMgdGhlIGNhc2UsIHRoZXkgYXJlIGF1dG9tYXRpY2FsbHkgZnJlZWQg d2hlbiBjaWZzX3JlYWRkYXRhIGlzIGZyZWVkLg0KDQpXaXRoIHRoZSBkaXJlY3QgSS9PIHBhdGNo LCB0aGUgcGFnZSBhcnJheSBpcyBoYW5kbGVkIHNlcGFyYXRlbHkuIFNvIHRoZXkgbmVlZCB0byBi ZSBmcmVlZCBhZnRlciBiZWluZyB1c2VkLg0KDQo+IA0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTog TG9uZyBMaSA8bG9uZ2xpQG1pY3Jvc29mdC5jb20+DQo+ID4gLS0tDQo+ID4gICBmcy9jaWZzL2Np ZnNnbG9iLmggfCAgMiArLQ0KPiA+ICAgZnMvY2lmcy9maWxlLmMgICAgIHwgMjMgKysrKysrKysr KysrKysrKysrKystLS0NCj4gPiAgIDIgZmlsZXMgY2hhbmdlZCwgMjEgaW5zZXJ0aW9ucygrKSwg NCBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9mcy9jaWZzL2NpZnNnbG9iLmgg Yi9mcy9jaWZzL2NpZnNnbG9iLmggaW5kZXgNCj4gPiA4ZDE2YzNlLi41Njg2NGE4NyAxMDA2NDQN Cj4gPiAtLS0gYS9mcy9jaWZzL2NpZnNnbG9iLmgNCj4gPiArKysgYi9mcy9jaWZzL2NpZnNnbG9i LmgNCj4gPiBAQCAtMTE3OSw3ICsxMTc5LDcgQEAgc3RydWN0IGNpZnNfcmVhZGRhdGEgew0KPiA+ ICAgCXVuc2lnbmVkIGludAkJCXRhaWxzejsNCj4gPiAgIAl1bnNpZ25lZCBpbnQJCQljcmVkaXRz Ow0KPiA+ICAgCXVuc2lnbmVkIGludAkJCW5yX3BhZ2VzOw0KPiA+IC0Jc3RydWN0IHBhZ2UJCQkq cGFnZXNbXTsNCj4gPiArCXN0cnVjdCBwYWdlCQkJKipwYWdlczsNCj4gDQo+IFRlY2huaWNhbGx5 IHNwZWFraW5nLCB0aGVzZSBhcmUgc3ludGFjdGljYWxseSBlcXVpdmFsZW50LiBJdCBtYXkgbm90 IGJlIHdvcnRoDQo+IGNoYW5naW5nIHRoaXMgaGlzdG9yaWMgZGVmaW5pdGlvbi4NCj4gDQo+IFRv bS4NCj4gDQo+IA0KPiA+ICAgfTsNCj4gPg0KPiA+ICAgc3RydWN0IGNpZnNfd3JpdGVkYXRhOw0K PiA+IGRpZmYgLS1naXQgYS9mcy9jaWZzL2ZpbGUuYyBiL2ZzL2NpZnMvZmlsZS5jIGluZGV4IDIz ZmQ0MzAuLjFjOTgyOTMNCj4gPiAxMDA2NDQNCj4gPiAtLS0gYS9mcy9jaWZzL2ZpbGUuYw0KPiA+ ICsrKyBiL2ZzL2NpZnMvZmlsZS5jDQo+ID4gQEAgLTI4ODAsMTMgKzI4ODAsMTMgQEAgY2lmc19z dHJpY3Rfd3JpdGV2KHN0cnVjdCBraW9jYiAqaW9jYiwgc3RydWN0DQo+IGlvdl9pdGVyICpmcm9t KQ0KPiA+ICAgfQ0KPiA+DQo+ID4gICBzdGF0aWMgc3RydWN0IGNpZnNfcmVhZGRhdGEgKg0KPiA+ IC1jaWZzX3JlYWRkYXRhX2FsbG9jKHVuc2lnbmVkIGludCBucl9wYWdlcywgd29ya19mdW5jX3Qg Y29tcGxldGUpDQo+ID4gK2NpZnNfcmVhZGRhdGFfZGlyZWN0X2FsbG9jKHN0cnVjdCBwYWdlICoq cGFnZXMsIHdvcmtfZnVuY190IGNvbXBsZXRlKQ0KPiA+ICAgew0KPiA+ICAgCXN0cnVjdCBjaWZz X3JlYWRkYXRhICpyZGF0YTsNCj4gPg0KPiA+IC0JcmRhdGEgPSBremFsbG9jKHNpemVvZigqcmRh dGEpICsgKHNpemVvZihzdHJ1Y3QgcGFnZSAqKSAqIG5yX3BhZ2VzKSwNCj4gPiAtCQkJR0ZQX0tF Uk5FTCk7DQo+ID4gKwlyZGF0YSA9IGt6YWxsb2Moc2l6ZW9mKCpyZGF0YSksIEdGUF9LRVJORUwp Ow0KPiA+ICAgCWlmIChyZGF0YSAhPSBOVUxMKSB7DQo+ID4gKwkJcmRhdGEtPnBhZ2VzID0gcGFn ZXM7DQo+ID4gICAJCWtyZWZfaW5pdCgmcmRhdGEtPnJlZmNvdW50KTsNCj4gPiAgIAkJSU5JVF9M SVNUX0hFQUQoJnJkYXRhLT5saXN0KTsNCj4gPiAgIAkJaW5pdF9jb21wbGV0aW9uKCZyZGF0YS0+ ZG9uZSk7DQo+ID4gQEAgLTI4OTYsNiArMjg5NiwyMiBAQCBjaWZzX3JlYWRkYXRhX2FsbG9jKHVu c2lnbmVkIGludCBucl9wYWdlcywNCj4gd29ya19mdW5jX3QgY29tcGxldGUpDQo+ID4gICAJcmV0 dXJuIHJkYXRhOw0KPiA+ICAgfQ0KPiA+DQo+ID4gK3N0YXRpYyBzdHJ1Y3QgY2lmc19yZWFkZGF0 YSAqDQo+ID4gK2NpZnNfcmVhZGRhdGFfYWxsb2ModW5zaWduZWQgaW50IG5yX3BhZ2VzLCB3b3Jr X2Z1bmNfdCBjb21wbGV0ZSkgew0KPiA+ICsJc3RydWN0IHBhZ2UgKipwYWdlcyA9DQo+ID4gKwkJ a3phbGxvYyhzaXplb2Yoc3RydWN0IHBhZ2UgKikgKiBucl9wYWdlcywgR0ZQX0tFUk5FTCk7DQo+ ID4gKwlzdHJ1Y3QgY2lmc19yZWFkZGF0YSAqcmV0ID0gTlVMTDsNCj4gPiArDQo+ID4gKwlpZiAo cGFnZXMpIHsNCj4gPiArCQlyZXQgPSBjaWZzX3JlYWRkYXRhX2RpcmVjdF9hbGxvYyhwYWdlcywg Y29tcGxldGUpOw0KPiA+ICsJCWlmICghcmV0KQ0KPiA+ICsJCQlrZnJlZShwYWdlcyk7DQo+ID4g Kwl9DQo+ID4gKw0KPiA+ICsJcmV0dXJuIHJldDsNCj4gPiArfQ0KPiA+ICsNCj4gPiAgIHZvaWQN Cj4gPiAgIGNpZnNfcmVhZGRhdGFfcmVsZWFzZShzdHJ1Y3Qga3JlZiAqcmVmY291bnQpDQo+ID4g ICB7DQo+ID4gQEAgLTI5MTAsNiArMjkyNiw3IEBAIGNpZnNfcmVhZGRhdGFfcmVsZWFzZShzdHJ1 Y3Qga3JlZiAqcmVmY291bnQpDQo+ID4gICAJaWYgKHJkYXRhLT5jZmlsZSkNCj4gPiAgIAkJY2lm c0ZpbGVJbmZvX3B1dChyZGF0YS0+Y2ZpbGUpOw0KPiA+DQo+ID4gKwlrdmZyZWUocmRhdGEtPnBh Z2VzKTsNCj4gPiAgIAlrZnJlZShyZGF0YSk7DQo+ID4gICB9DQo+ID4NCj4gPg0K -- 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
On Sat, Jun 23, 2018 at 09:50:20PM -0400, Tom Talpey wrote: > On 5/30/2018 3:47 PM, Long Li wrote: > >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. > > "Caller" is still responsible? Or is the rdata somehow freeing itself > via another mechanism? > > > > >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 > >+++ 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; > > Technically speaking, these are syntactically equivalent. It may not > be worth changing this historic definition. [] is a C99 'flex array', it has a different allocation behavior than ** and is not interchangeable.. Jason -- 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
On 6/25/2018 5:01 PM, Jason Gunthorpe wrote: > On Sat, Jun 23, 2018 at 09:50:20PM -0400, Tom Talpey wrote: >> On 5/30/2018 3:47 PM, Long Li wrote: >>> 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. >> >> "Caller" is still responsible? Or is the rdata somehow freeing itself >> via another mechanism? >> >>> >>> 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 >>> +++ 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; >> >> Technically speaking, these are syntactically equivalent. It may not >> be worth changing this historic definition. > > [] is a C99 'flex array', it has a different allocation behavior than > ** and is not interchangeable.. In that case, it's an even better reason to not change the declaration. Tom. -- 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
PiBTdWJqZWN0OiBSZTogW1BhdGNoIHYyIDAyLzE1XSBDSUZTOiBBZGQgc3VwcG9ydCBmb3IgZGly ZWN0IHBhZ2VzIGluIHJkYXRhDQo+IA0KPiBPbiA2LzI1LzIwMTggNTowMSBQTSwgSmFzb24gR3Vu dGhvcnBlIHdyb3RlOg0KPiA+IE9uIFNhdCwgSnVuIDIzLCAyMDE4IGF0IDA5OjUwOjIwUE0gLTA0 MDAsIFRvbSBUYWxwZXkgd3JvdGU6DQo+ID4+IE9uIDUvMzAvMjAxOCAzOjQ3IFBNLCBMb25nIExp IHdyb3RlOg0KPiA+Pj4gRnJvbTogTG9uZyBMaSA8bG9uZ2xpQG1pY3Jvc29mdC5jb20+DQo+ID4+ Pg0KPiA+Pj4gQWRkIGEgZnVuY3Rpb24gdG8gYWxsb2NhdGUgcmRhdGEgd2l0aG91dCBhbGxvY2F0 aW5nIHBhZ2VzIGZvciBkYXRhDQo+ID4+PiB0cmFuc2Zlci4gVGhpcyBnaXZlcyB0aGUgY2FsbGVy IGFuIG9wdGlvbiB0byBwYXNzIGEgbnVtYmVyIG9mIHBhZ2VzDQo+ID4+PiB0aGF0IHBvaW50IHRv IHRoZSBkYXRhIGJ1ZmZlci4NCj4gPj4+DQo+ID4+PiByZGF0YSBpcyBzdGlsbCByZXBvbnNpYmxl IGZvciBmcmVlIHRob3NlIHBhZ2VzIGFmdGVyIGl0J3MgZG9uZS4NCj4gPj4NCj4gPj4gIkNhbGxl ciIgaXMgc3RpbGwgcmVzcG9uc2libGU/IE9yIGlzIHRoZSByZGF0YSBzb21laG93IGZyZWVpbmcg aXRzZWxmDQo+ID4+IHZpYSBhbm90aGVyIG1lY2hhbmlzbT8NCj4gPj4NCj4gPj4+DQo+ID4+PiBT aWduZWQtb2ZmLWJ5OiBMb25nIExpIDxsb25nbGlAbWljcm9zb2Z0LmNvbT4NCj4gPj4+ICAgZnMv Y2lmcy9jaWZzZ2xvYi5oIHwgIDIgKy0NCj4gPj4+ICAgZnMvY2lmcy9maWxlLmMgICAgIHwgMjMg KysrKysrKysrKysrKysrKysrKystLS0NCj4gPj4+ICAgMiBmaWxlcyBjaGFuZ2VkLCAyMSBpbnNl cnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPiA+Pj4NCj4gPj4+IGRpZmYgLS1naXQgYS9mcy9j aWZzL2NpZnNnbG9iLmggYi9mcy9jaWZzL2NpZnNnbG9iLmggaW5kZXgNCj4gPj4+IDhkMTZjM2Uu LjU2ODY0YTg3IDEwMDY0NA0KPiA+Pj4gKysrIGIvZnMvY2lmcy9jaWZzZ2xvYi5oDQo+ID4+PiBA QCAtMTE3OSw3ICsxMTc5LDcgQEAgc3RydWN0IGNpZnNfcmVhZGRhdGEgew0KPiA+Pj4gICAJdW5z aWduZWQgaW50CQkJdGFpbHN6Ow0KPiA+Pj4gICAJdW5zaWduZWQgaW50CQkJY3JlZGl0czsNCj4g Pj4+ICAgCXVuc2lnbmVkIGludAkJCW5yX3BhZ2VzOw0KPiA+Pj4gLQlzdHJ1Y3QgcGFnZQkJCSpw YWdlc1tdOw0KPiA+Pj4gKwlzdHJ1Y3QgcGFnZQkJCSoqcGFnZXM7DQo+ID4+DQo+ID4+IFRlY2hu aWNhbGx5IHNwZWFraW5nLCB0aGVzZSBhcmUgc3ludGFjdGljYWxseSBlcXVpdmFsZW50LiBJdCBt YXkgbm90DQo+ID4+IGJlIHdvcnRoIGNoYW5naW5nIHRoaXMgaGlzdG9yaWMgZGVmaW5pdGlvbi4N Cj4gPg0KPiA+IFtdIGlzIGEgQzk5ICdmbGV4IGFycmF5JywgaXQgaGFzIGEgZGlmZmVyZW50IGFs bG9jYXRpb24gYmVoYXZpb3IgdGhhbg0KPiA+ICoqIGFuZCBpcyBub3QgaW50ZXJjaGFuZ2VhYmxl Li4NCj4gDQo+IEluIHRoYXQgY2FzZSwgaXQncyBhbiBldmVuIGJldHRlciByZWFzb24gdG8gbm90 IGNoYW5nZSB0aGUgZGVjbGFyYXRpb24uDQoNCk5vLCBpdCBuZWVkcyB0byBiZSBkZWNsYXJlZCBz ZXBhcmF0ZWx5Lg0KDQpXaXRoIERpcmVjdCBJL08sICoqcGFnZXMgYXJlIGFsbG9jYXRlZCBhbmQg cmV0dXJuZWQgZnJvbSBpb3ZfaXRlcl9nZXRfcGFnZXNfYWxsb2MoKSB3aGVuIGxvY2tpbmcgdGhv c2UgdXNlciBwYWdlcy4gVGhleSBjYW4ndCBiZSBhbGxvY2F0ZWQgYXMgcGFydCBvZiBzdHJ1Y3Qg Y2lmc19yZWFkZGF0YS4NCg0K -- 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
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); }