* [PATCH 1/2] vfs: switch iov_iter_get_pages() to passing maximal size
2014-09-03 10:10 [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Maxim Patlasov
@ 2014-09-03 10:11 ` Maxim Patlasov
2014-09-03 10:11 ` [PATCH 2/2] fuse: fuse_get_user_pages(): do not pack more data than requested Maxim Patlasov
2014-09-10 9:51 ` [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Miklos Szeredi
2 siblings, 0 replies; 5+ messages in thread
From: Maxim Patlasov @ 2014-09-03 10:11 UTC (permalink / raw)
To: viro
Cc: miklos, fuse-devel, linux-kernel, linux-mm, linux-fsdevel,
werner.baumann
The patch reverts the commit c7f3888ad7f0 ("switch iov_iter_get_pages() to
passing maximal number of pages") because FUSE do need maxsize argument that
existed before that commit.
Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com>
---
fs/direct-io.c | 2 +-
fs/fuse/file.c | 4 ++--
include/linux/uio.h | 2 +-
mm/iov_iter.c | 17 +++++++++--------
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index c311640..17e39b0 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -158,7 +158,7 @@ static inline int dio_refill_pages(struct dio *dio, struct dio_submit *sdio)
{
ssize_t ret;
- ret = iov_iter_get_pages(sdio->iter, dio->pages, DIO_PAGES,
+ ret = iov_iter_get_pages(sdio->iter, dio->pages, DIO_PAGES * PAGE_SIZE,
&sdio->from);
if (ret < 0 && sdio->blocks_available && (dio->rw & WRITE)) {
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 912061a..40ac262 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1303,10 +1303,10 @@ static int fuse_get_user_pages(struct fuse_req *req, struct iov_iter *ii,
while (nbytes < *nbytesp && req->num_pages < req->max_pages) {
unsigned npages;
size_t start;
+ unsigned n = req->max_pages - req->num_pages;
ssize_t ret = iov_iter_get_pages(ii,
&req->pages[req->num_pages],
- req->max_pages - req->num_pages,
- &start);
+ n * PAGE_SIZE, &start);
if (ret < 0)
return ret;
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 48d64e6..09a7cff 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -84,7 +84,7 @@ unsigned long iov_iter_alignment(const struct iov_iter *i);
void iov_iter_init(struct iov_iter *i, int direction, const struct iovec *iov,
unsigned long nr_segs, size_t count);
ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
- unsigned maxpages, size_t *start);
+ size_t maxsize, size_t *start);
ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
size_t maxsize, size_t *start);
int iov_iter_npages(const struct iov_iter *i, int maxpages);
diff --git a/mm/iov_iter.c b/mm/iov_iter.c
index ab88dc0..7b5dbd1 100644
--- a/mm/iov_iter.c
+++ b/mm/iov_iter.c
@@ -310,7 +310,7 @@ void iov_iter_init(struct iov_iter *i, int direction,
EXPORT_SYMBOL(iov_iter_init);
static ssize_t get_pages_iovec(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize,
size_t *start)
{
size_t offset = i->iov_offset;
@@ -323,10 +323,10 @@ static ssize_t get_pages_iovec(struct iov_iter *i,
len = iov->iov_len - offset;
if (len > i->count)
len = i->count;
+ if (len > maxsize)
+ len = maxsize;
addr = (unsigned long)iov->iov_base + offset;
len += *start = addr & (PAGE_SIZE - 1);
- if (len > maxpages * PAGE_SIZE)
- len = maxpages * PAGE_SIZE;
addr &= ~(PAGE_SIZE - 1);
n = (len + PAGE_SIZE - 1) / PAGE_SIZE;
res = get_user_pages_fast(addr, n, (i->type & WRITE) != WRITE, pages);
@@ -588,14 +588,15 @@ static unsigned long alignment_bvec(const struct iov_iter *i)
}
static ssize_t get_pages_bvec(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize,
size_t *start)
{
const struct bio_vec *bvec = i->bvec;
size_t len = bvec->bv_len - i->iov_offset;
if (len > i->count)
len = i->count;
- /* can't be more than PAGE_SIZE */
+ if (len > maxsize)
+ len = maxsize;
*start = bvec->bv_offset + i->iov_offset;
get_page(*pages = bvec->bv_page);
@@ -711,13 +712,13 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)
EXPORT_SYMBOL(iov_iter_alignment);
ssize_t iov_iter_get_pages(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize,
size_t *start)
{
if (i->type & ITER_BVEC)
- return get_pages_bvec(i, pages, maxpages, start);
+ return get_pages_bvec(i, pages, maxsize, start);
else
- return get_pages_iovec(i, pages, maxpages, start);
+ return get_pages_iovec(i, pages, maxsize, start);
}
EXPORT_SYMBOL(iov_iter_get_pages);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] fuse: fuse_get_user_pages(): do not pack more data than requested
2014-09-03 10:10 [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Maxim Patlasov
2014-09-03 10:11 ` [PATCH 1/2] vfs: switch iov_iter_get_pages() to passing maximal size Maxim Patlasov
@ 2014-09-03 10:11 ` Maxim Patlasov
2014-09-10 9:51 ` [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Miklos Szeredi
2 siblings, 0 replies; 5+ messages in thread
From: Maxim Patlasov @ 2014-09-03 10:11 UTC (permalink / raw)
To: viro
Cc: miklos, fuse-devel, linux-kernel, linux-mm, linux-fsdevel,
werner.baumann
The patch fixes a bug introduced by commit c9c37e2e6378 ("fuse: switch to
iov_iter_get_pages()"):
The third argument of fuse_get_user_pages() "nbytesp" refers to the number of
bytes a caller asked to pack into fuse request. This value may be lesser
than capacity of fuse request or iov_iter. So fuse_get_user_pages() must
ensure that *nbytesp won't grow. Before that commit, it was ensured by:
> ret = get_user_pages_fast(user_addr, npages, !write,
> &req->pages[req->num_pages]);
> ...
> npages = ret;
> frag_size = min_t(size_t, frag_size,
> (npages << PAGE_SHIFT) - offset);
Now, when helper iov_iter_get_pages() performs all hard work of extracting
pages from iov_iter, it can be done by passing properly calculated "maxsize"
to the helper.
Reported-by: Werner Baumann <werner.baumann@onlinehome.de>
Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com>
---
fs/fuse/file.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 40ac262..1d2bb70 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1303,10 +1303,15 @@ static int fuse_get_user_pages(struct fuse_req *req, struct iov_iter *ii,
while (nbytes < *nbytesp && req->num_pages < req->max_pages) {
unsigned npages;
size_t start;
+ ssize_t ret;
unsigned n = req->max_pages - req->num_pages;
- ssize_t ret = iov_iter_get_pages(ii,
- &req->pages[req->num_pages],
- n * PAGE_SIZE, &start);
+ size_t frag_size = fuse_get_frag_size(ii, *nbytesp - nbytes);
+
+ frag_size = min_t(size_t, frag_size, n << PAGE_SHIFT);
+
+ ret = iov_iter_get_pages(ii,
+ &req->pages[req->num_pages],
+ frag_size, &start);
if (ret < 0)
return ret;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] fuse: fix regression in fuse_get_user_pages()
2014-09-03 10:10 [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Maxim Patlasov
2014-09-03 10:11 ` [PATCH 1/2] vfs: switch iov_iter_get_pages() to passing maximal size Maxim Patlasov
2014-09-03 10:11 ` [PATCH 2/2] fuse: fuse_get_user_pages(): do not pack more data than requested Maxim Patlasov
@ 2014-09-10 9:51 ` Miklos Szeredi
2014-09-10 12:24 ` Maxim Patlasov
2 siblings, 1 reply; 5+ messages in thread
From: Miklos Szeredi @ 2014-09-10 9:51 UTC (permalink / raw)
To: Maxim Patlasov
Cc: viro, fuse-devel, linux-kernel, linux-mm, linux-fsdevel,
werner.baumann
On Wed, Sep 03, 2014 at 02:10:23PM +0400, Maxim Patlasov wrote:
> Hi,
>
> The patchset fixes a regression introduced by the following commits:
>
> c7f3888ad7f0 ("switch iov_iter_get_pages() to passing maximal number of pages")
> c9c37e2e6378 ("fuse: switch to iov_iter_get_pages()")
>
Hmm, instead of reverting to passing maxbytes *instead* of maxpages, I think the
right fix is to *add* the maxbytes argument.
Just maxbytes alone doesn't have enough information in it. E.g. 4096 contiguous
bytes could occupy 1 or 2 pages, depending on the starting offset.
So how about the following (untested) patch?
Thanks,
Miklos
diff --git a/fs/direct-io.c b/fs/direct-io.c
index c3116404ab49..e181b6b2e297 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -158,7 +158,7 @@ static inline int dio_refill_pages(struct dio *dio, struct dio_submit *sdio)
{
ssize_t ret;
- ret = iov_iter_get_pages(sdio->iter, dio->pages, DIO_PAGES,
+ ret = iov_iter_get_pages(sdio->iter, dio->pages, LONG_MAX, DIO_PAGES,
&sdio->from);
if (ret < 0 && sdio->blocks_available && (dio->rw & WRITE)) {
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 912061ac4baf..caa8d95b24e8 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1305,6 +1305,7 @@ static int fuse_get_user_pages(struct fuse_req *req, struct iov_iter *ii,
size_t start;
ssize_t ret = iov_iter_get_pages(ii,
&req->pages[req->num_pages],
+ *nbytesp - nbytes,
req->max_pages - req->num_pages,
&start);
if (ret < 0)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 48d64e6ab292..290fbf0b6b8a 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -84,7 +84,7 @@ unsigned long iov_iter_alignment(const struct iov_iter *i);
void iov_iter_init(struct iov_iter *i, int direction, const struct iovec *iov,
unsigned long nr_segs, size_t count);
ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
- unsigned maxpages, size_t *start);
+ size_t maxsize, unsigned maxpages, size_t *start);
ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
size_t maxsize, size_t *start);
int iov_iter_npages(const struct iov_iter *i, int maxpages);
diff --git a/mm/iov_iter.c b/mm/iov_iter.c
index ab88dc0ea1d3..9a09f2034fcc 100644
--- a/mm/iov_iter.c
+++ b/mm/iov_iter.c
@@ -310,7 +310,7 @@ void iov_iter_init(struct iov_iter *i, int direction,
EXPORT_SYMBOL(iov_iter_init);
static ssize_t get_pages_iovec(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize, unsigned maxpages,
size_t *start)
{
size_t offset = i->iov_offset;
@@ -323,6 +323,8 @@ static ssize_t get_pages_iovec(struct iov_iter *i,
len = iov->iov_len - offset;
if (len > i->count)
len = i->count;
+ if (len > maxsize)
+ len = maxsize;
addr = (unsigned long)iov->iov_base + offset;
len += *start = addr & (PAGE_SIZE - 1);
if (len > maxpages * PAGE_SIZE)
@@ -588,13 +590,15 @@ static unsigned long alignment_bvec(const struct iov_iter *i)
}
static ssize_t get_pages_bvec(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize, unsigned maxpages,
size_t *start)
{
const struct bio_vec *bvec = i->bvec;
size_t len = bvec->bv_len - i->iov_offset;
if (len > i->count)
len = i->count;
+ if (len > maxsize)
+ len = maxsize;
/* can't be more than PAGE_SIZE */
*start = bvec->bv_offset + i->iov_offset;
@@ -711,13 +715,13 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)
EXPORT_SYMBOL(iov_iter_alignment);
ssize_t iov_iter_get_pages(struct iov_iter *i,
- struct page **pages, unsigned maxpages,
+ struct page **pages, size_t maxsize, unsigned maxpages,
size_t *start)
{
if (i->type & ITER_BVEC)
- return get_pages_bvec(i, pages, maxpages, start);
+ return get_pages_bvec(i, pages, maxsize, maxpages, start);
else
- return get_pages_iovec(i, pages, maxpages, start);
+ return get_pages_iovec(i, pages, maxsize, maxpages, start);
}
EXPORT_SYMBOL(iov_iter_get_pages);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] fuse: fix regression in fuse_get_user_pages()
2014-09-10 9:51 ` [PATCH 0/2] fuse: fix regression in fuse_get_user_pages() Miklos Szeredi
@ 2014-09-10 12:24 ` Maxim Patlasov
0 siblings, 0 replies; 5+ messages in thread
From: Maxim Patlasov @ 2014-09-10 12:24 UTC (permalink / raw)
To: Miklos Szeredi
Cc: viro, fuse-devel, linux-kernel, linux-mm, linux-fsdevel,
werner.baumann
On 09/10/2014 01:51 PM, Miklos Szeredi wrote:
> On Wed, Sep 03, 2014 at 02:10:23PM +0400, Maxim Patlasov wrote:
>> Hi,
>>
>> The patchset fixes a regression introduced by the following commits:
>>
>> c7f3888ad7f0 ("switch iov_iter_get_pages() to passing maximal number of pages")
>> c9c37e2e6378 ("fuse: switch to iov_iter_get_pages()")
>>
> Hmm, instead of reverting to passing maxbytes *instead* of maxpages, I think the
> right fix is to *add* the maxbytes argument.
>
> Just maxbytes alone doesn't have enough information in it. E.g. 4096 contiguous
> bytes could occupy 1 or 2 pages, depending on the starting offset.
Yes, you are right. I missed that c7f3888ad7f0 fixed a subtle bug in
get_pages_iovec().
>
> So how about the following (untested) patch?
Your patch works fine in my tests.
Thanks,
Maxim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread