Messages in this thread |  | | Date | Mon, 26 Jul 2021 14:17:02 +0200 | From | Christoph Hellwig <> | Subject | Re: [PATCH v7] iomap: make inline data support more flexible |
| |
> Subject: iomap: Support tail packing
I can't say I like this "tail packing" language here when we have the perfectly fine inline wording. Same for various comments in the actual code.
> + /* inline and tail-packed data must start page aligned in the file */ > + if (WARN_ON_ONCE(offset_in_page(iomap->offset))) > + return -EIO; > + if (WARN_ON_ONCE(size > PAGE_SIZE - offset_in_page(iomap->inline_data))) > + return -EIO;
Why can't we use iomap_inline_data_size_valid here? That is how can size be different from iomap->legth?
Shouldn't the offset_in_page also go into iomap_inline_data_size_valid, which should probably be called iomap_inline_data_valid then?
> if (iomap->type == IOMAP_INLINE) { > + int ret = iomap_read_inline_data(inode, page, iomap); > + return ret ?: PAGE_SIZE;
The ?: expression without the first leg is really confuing. Especially if a good old if is much more readable here.
int ret = iomap_read_inline_data(inode, page, iomap);
if (ret) return ret; return PAGE_SIZE;
> + copied = copy_from_iter(iomap_inline_data(iomap, pos), length, iter);
> + copied = copy_to_iter(iomap_inline_data(iomap, pos), length, iter);
Pleae avoid the overly long lines.
|  |