public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: Richard Palethorpe <rpalethorpe@suse.de>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v1] readahead02.c: Use fsync instead of sync
Date: Tue, 17 Jan 2023 17:33:36 +0100	[thread overview]
Message-ID: <Y8bN4Bxkook8BZvs@yuki> (raw)
In-Reply-To: <877cxlwk66.fsf@suse.de>

Hi!
> > The motivation of this change is base on the https://github.com/linux-test-project/ltp/issues/972
> > which give following suggestion:
> > "As we run the test inside a loop device I guess that we can also 
> > sync and drop caches just for the device, which should be faster 
> > than syncing and dropping the whole system. Possibly we just need 
> > to umount it and mount it again."
> 
> I see. Well unless Cyril can show that the test is actually failing
> somewhere (or there is a strong logical argument this will cause a
> failure). Then this task is still valid, but low priority IMO.

We do sync more than needed here, since we are looking at the per device
counters we have to sync just the device we mount for the test, so this
is optimization for the case that the system has many dirty cases and
will need seconds or a minute to write them to the pernament storage.

> > But currently i can not find any API to sync and drop caches just 
> > ONLY for device, so base my view just replace sync whole 
> > system to single file also can make a small help.
> 
> If we don't have one or more concrete failures to focus on then we
> really have to research whether fsync (or syncfs FYI) or unmounting the
> device are the correct thing to do. They will all have subtly different
> effects.

Looking at the code closely I'm starting to think that the sync is not
required at all. What we do in the test is that we create file and sync
it to the external storage. Then we read it a few times and mesure
differences in cache. As far as I can tell we just need to drop the page
cache after we have read the file. What do you think?

In any case I would avoid changing the test before the release, but it's
certainly something we can look at after that.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

  reply	other threads:[~2023-01-17 16:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-16  7:41 [LTP] [PATCH v1] readahead02.c: Use fsync instead of sync Wei Gao via ltp
2023-01-16 15:08 ` Richard Palethorpe
2023-01-17  2:22   ` Wei Gao via ltp
2023-01-17  9:23     ` Richard Palethorpe
2023-01-17 16:33       ` Cyril Hrubis [this message]
2023-01-17 16:50         ` Richard Palethorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y8bN4Bxkook8BZvs@yuki \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    --cc=rpalethorpe@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox