On Fri, May 4, 2018 at 6:05 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Fri, 2018-05-04 at 17:04 +0200, Ilya Dryomov wrote:
>> dio_get_pagev_size() and dio_get_pages_alloc() introduced in commit
>> b5b98989dc7e ("ceph: combine as many iovec as possile into one OSD
>> request") assume that the passed iov_iter is ITER_IOVEC. This isn't
>> the case with splice where it ends up poking into the guts of ITER_BVEC
>> or ITER_PIPE iterators, causing lockups and crashes easily reproduced
>> with generic/095.
>>
>> Rather than trying to figure out gap alignment and stuff pages into
>> a page vector, add a helper for going from iov_iter to a bio_vec array
>> and make use of the new CEPH_OSD_DATA_TYPE_BVECS code.
>>
>> Fixes: b5b98989dc7e ("ceph: combine as many iovec as possile into one OSD request")
>> Link: http://tracker.ceph.com/issues/18130
>> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
>> ---
>> fs/ceph/file.c | 193 +++++++++++++++++++++++++++++++++------------------------
>> 1 file changed, 113 insertions(+), 80 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 8ce7849f3fbd..cb5daff99816 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -69,70 +69,99 @@ static __le32 ceph_flags_sys2wire(u32 flags)
>> * need to wait for MDS acknowledgement.
>> */
>>
>> -/*
>> - * Calculate the length sum of direct io vectors that can
>> - * be combined into one page vector.
>> - */
>> -static size_t dio_get_pagev_size(const struct iov_iter *it)
>> +static ssize_t __iter_get_bvecs(struct iov_iter *iter, size_t maxsize,
>> + struct bio_vec *bvecs)
>
> Can you add a comment explaining what this does, and what it returns? I
> assume it returns the number of bytes represented by the resulting bvec
> array. Maybe also mention that the bvec array must be preallocated.
iter_get_bvecs_alloc() comment below explains it. __iter_get_bvecs()
is a private helper -- I didn't see a reason to duplicate most of that.
>
>> {
>> - const struct iovec *iov = it->iov;
>> - const struct iovec *iovend = iov + it->nr_segs;
>> - size_t size;
>> -
>> - size = iov->iov_len - it->iov_offset;
>> - /*
>> - * An iov can be page vectored when both the current tail
>> - * and the next base are page aligned.
>> - */
>> - while (PAGE_ALIGNED((iov->iov_base + iov->iov_len)) &&
>> - (++iov < iovend && PAGE_ALIGNED((iov->iov_base)))) {
>> - size += iov->iov_len;
>> - }
>> - dout("dio_get_pagevlen len = %zu\n", size);
>> - return size;
>> + size_t size = 0;
>> + int bvec_idx = 0;
>> +
>> + if (maxsize > iov_iter_count(iter))
>> + maxsize = iov_iter_count(iter);
>> +
>> + while (size < maxsize) {
>> + struct page *pages[64]; /* DIO_PAGES */
>
> Why 64 here? Magic numbers are nasty. Can you #define a constant and
> plug it in here, and in the iov_iter_get_pages call below?
Added
/*
* How many pages to get in one call to iov_iter_get_pages(). This
* determines the size of the on-stack array used as a buffer.
*/
#define ITER_GET_BVECS_PAGES 64
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
