2024-05-15 06:04:03

by Daniel Gomez

[permalink] [raw]
Subject: [PATCH 07/12] shmem: check if a block is uptodate before splice into pipe

The splice_read() path assumes folios are always uptodate. Make sure
all blocks in the given range are uptodate or else, splice zeropage into
the pipe. Maximize the number of blocks that can be spliced into pipe at
once by increasing the 'part' to the latest uptodate block found.

Signed-off-by: Daniel Gomez <[email protected]>
---
mm/shmem.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 68fe769d91b1..e06cb6438ef8 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3223,8 +3223,30 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
if (unlikely(*ppos >= isize))
break;
part = min_t(loff_t, isize - *ppos, len);
+ if (folio && folio_test_large(folio) &&
+ folio_test_private(folio)) {
+ unsigned long from = offset_in_folio(folio, *ppos);
+ unsigned int bfirst = from >> inode->i_blkbits;
+ unsigned int blast, blast_upd;
+
+ len = min(folio_size(folio) - from, len);
+ blast = (from + len - 1) >> inode->i_blkbits;
+
+ blast_upd = sfs_get_last_block_uptodate(folio, bfirst,
+ blast);
+ if (blast_upd <= blast) {
+ unsigned int bsize = 1 << inode->i_blkbits;
+ unsigned int blks = blast_upd - bfirst + 1;
+ unsigned int bbytes = blks << inode->i_blkbits;
+ unsigned int boff = (*ppos % bsize);
+
+ part = min_t(loff_t, bbytes - boff, len);
+ }
+ }

- if (folio) {
+ if (folio && shmem_is_block_uptodate(
+ folio, offset_in_folio(folio, *ppos) >>
+ inode->i_blkbits)) {
/*
* If users can be writing to this page using arbitrary
* virtual addresses, take care about potential aliasing
--
2.43.0


2024-05-16 13:20:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 07/12] shmem: check if a block is uptodate before splice into pipe

Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on xfs-linux/for-next brauner-vfs/vfs.all linus/master v6.9 next-20240516]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Gomez/splice-don-t-check-for-uptodate-if-partially-uptodate-is-impl/20240515-135925
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20240515055719.32577-8-da.gomez%40samsung.com
patch subject: [PATCH 07/12] shmem: check if a block is uptodate before splice into pipe
config: arm-s5pv210_defconfig (https://download.01.org/0day-ci/archive/20240516/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240516/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

arm-linux-gnueabi-ld: mm/shmem.o: in function `shmem_file_splice_read':
>> mm/shmem.c:3240:(.text+0x5224): undefined reference to `__aeabi_ldivmod'


vim +3240 mm/shmem.c

3174
3175 static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
3176 struct pipe_inode_info *pipe,
3177 size_t len, unsigned int flags)
3178 {
3179 struct inode *inode = file_inode(in);
3180 struct address_space *mapping = inode->i_mapping;
3181 struct folio *folio = NULL;
3182 size_t total_spliced = 0, used, npages, n, part;
3183 loff_t isize;
3184 int error = 0;
3185
3186 /* Work out how much data we can actually add into the pipe */
3187 used = pipe_occupancy(pipe->head, pipe->tail);
3188 npages = max_t(ssize_t, pipe->max_usage - used, 0);
3189 len = min_t(size_t, len, npages * PAGE_SIZE);
3190
3191 do {
3192 if (*ppos >= i_size_read(inode))
3193 break;
3194
3195 error = shmem_get_folio(inode, *ppos / PAGE_SIZE, &folio,
3196 SGP_READ);
3197 if (error) {
3198 if (error == -EINVAL)
3199 error = 0;
3200 break;
3201 }
3202 if (folio) {
3203 folio_unlock(folio);
3204
3205 if (folio_test_hwpoison(folio) ||
3206 (folio_test_large(folio) &&
3207 folio_test_has_hwpoisoned(folio))) {
3208 error = -EIO;
3209 break;
3210 }
3211 }
3212
3213 /*
3214 * i_size must be checked after we know the pages are Uptodate.
3215 *
3216 * Checking i_size after the check allows us to calculate
3217 * the correct value for "nr", which means the zero-filled
3218 * part of the page is not copied back to userspace (unless
3219 * another truncate extends the file - this is desired though).
3220 */
3221 isize = i_size_read(inode);
3222 if (unlikely(*ppos >= isize))
3223 break;
3224 part = min_t(loff_t, isize - *ppos, len);
3225 if (folio && folio_test_large(folio) &&
3226 folio_test_private(folio)) {
3227 unsigned long from = offset_in_folio(folio, *ppos);
3228 unsigned int bfirst = from >> inode->i_blkbits;
3229 unsigned int blast, blast_upd;
3230
3231 len = min(folio_size(folio) - from, len);
3232 blast = (from + len - 1) >> inode->i_blkbits;
3233
3234 blast_upd = sfs_get_last_block_uptodate(folio, bfirst,
3235 blast);
3236 if (blast_upd <= blast) {
3237 unsigned int bsize = 1 << inode->i_blkbits;
3238 unsigned int blks = blast_upd - bfirst + 1;
3239 unsigned int bbytes = blks << inode->i_blkbits;
> 3240 unsigned int boff = (*ppos % bsize);
3241
3242 part = min_t(loff_t, bbytes - boff, len);
3243 }
3244 }
3245
3246 if (folio && shmem_is_block_uptodate(
3247 folio, offset_in_folio(folio, *ppos) >>
3248 inode->i_blkbits)) {
3249 /*
3250 * If users can be writing to this page using arbitrary
3251 * virtual addresses, take care about potential aliasing
3252 * before reading the page on the kernel side.
3253 */
3254 if (mapping_writably_mapped(mapping))
3255 flush_dcache_folio(folio);
3256 folio_mark_accessed(folio);
3257 /*
3258 * Ok, we have the page, and it's up-to-date, so we can
3259 * now splice it into the pipe.
3260 */
3261 n = splice_folio_into_pipe(pipe, folio, *ppos, part);
3262 folio_put(folio);
3263 folio = NULL;
3264 } else {
3265 n = splice_zeropage_into_pipe(pipe, *ppos, part);
3266 }
3267
3268 if (!n)
3269 break;
3270 len -= n;
3271 total_spliced += n;
3272 *ppos += n;
3273 in->f_ra.prev_pos = *ppos;
3274 if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
3275 break;
3276
3277 cond_resched();
3278 } while (len);
3279
3280 if (folio)
3281 folio_put(folio);
3282
3283 file_accessed(in);
3284 return total_spliced ? total_spliced : error;
3285 }
3286

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki