Message ID | 446f0df5-798d-ab3a-e773-39d9f202c092@virtuozzo.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fuse_writepages_fill: simplified "if-else if" constuction | expand |
On Thu, Jun 25, 2020 at 11:30 AM Vasily Averin <vvs@virtuozzo.com> wrote: > > fuse_writepages_fill uses following construction: > if (wpa && ap->num_pages && > (A || B || C)) { > action; > } else if (wpa && D) { > if (E) { > the same action; > } > } > > - ap->num_pages check is always true and can be removed > - "if" and "else if" calls the same action and can be merged. Makes sense. Attached patch goes further and moves checking the conditions to a separate helper for clarity. Thanks, Miklos
On 7/14/20 3:24 PM, Miklos Szeredi wrote: > On Thu, Jun 25, 2020 at 11:30 AM Vasily Averin <vvs@virtuozzo.com> wrote: >> >> fuse_writepages_fill uses following construction: >> if (wpa && ap->num_pages && >> (A || B || C)) { >> action; >> } else if (wpa && D) { >> if (E) { >> the same action; >> } >> } >> >> - ap->num_pages check is always true and can be removed >> - "if" and "else if" calls the same action and can be merged. > > Makes sense. Attached patch goes further and moves checking the > conditions to a separate helper for clarity. This looks perfect for me, thank you Vasily Averin
diff --git a/fs/fuse/file.c b/fs/fuse/file.c index cf267bd..c023f7f0 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2035,17 +2035,14 @@ static int fuse_writepages_fill(struct page *page, */ is_writeback = fuse_page_is_writeback(inode, page->index); - if (wpa && ap->num_pages && + if (wpa && (is_writeback || ap->num_pages == fc->max_pages || (ap->num_pages + 1) * PAGE_SIZE > fc->max_write || - data->orig_pages[ap->num_pages - 1]->index + 1 != page->index)) { + (data->orig_pages[ap->num_pages - 1]->index + 1 != page->index) || + ((ap->num_pages == data->max_pages) && + (!fuse_pages_realloc(data))))) { fuse_writepages_send(data); data->wpa = NULL; - } else if (wpa && ap->num_pages == data->max_pages) { - if (!fuse_pages_realloc(data)) { - fuse_writepages_send(data); - data->wpa = NULL; - } } err = -ENOMEM;
fuse_writepages_fill uses following construction: if (wpa && ap->num_pages && (A || B || C)) { action; } else if (wpa && D) { if (E) { the same action; } } - ap->num_pages check is always true and can be removed - "if" and "else if" calls the same action and can be merged. Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- fs/fuse/file.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)