linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sougata Santra <sougata@tuxera.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <hch@infradead.org>, <linux-fsdevel@vger.kernel.org>,
	Vyacheslav Dubeyko <slava@dubeyko.com>,
	Fabian Frederick <fabf@skynet.be>
Subject: Re: [PATCH 1/1] hfsplus: skip unnecessary volume header sync
Date: Fri, 18 Jul 2014 11:35:37 +0300	[thread overview]
Message-ID: <1405672537.25052.18.camel@ultrabook> (raw)
In-Reply-To: <20140717135929.1a9fa7279cbb4e7a57761213@linux-foundation.org>

On Thu, 2014-07-17 at 13:59 -0700, Andrew Morton wrote:
> On Thu, 17 Jul 2014 19:32:42 +0300 Sougata Santra <sougata@tuxera.com> wrote:
> 
> > hfsplus_sync_fs always updates volume header information to disk with every
> > sync. This causes problem for systems trying to monitor disk activity to
> > switch them to low power state.
> 
> Please fully describe these "problems"?  I'm guessing that the fs will
> write the volume header even if it didn't change, so a sync operation
> against a completely clean fs still performs a physical write.  But I'd
> prefer not to have to guess!

Absolutely sorry, I assumed it was made clear from the cover letter.
Yes, as you described, the problem is that HFS+ syncs volume header
even if it did not change with every sync operation. Also, I have
done some additional changes because I was modifying hfsplus_sync_fs,
and the problems were relevant, and I thought they should be addressed
in the same patch. Please find them below:

Please Note:
-----------
1) hfsplus_sync_volume_header() is added to call from mount/unmount
sequence, since we just want to write the dirty/clean state to disk.
For unmount, hfsplus_sync_fs is already called from sync_filesystem(). 
For mount, it gets called from delayed_sync_fs().

2) Also, there was a error in error propagation. It it also fixed in 
this patch.
-->snip<--
if (!error)
        error2 = error;
-->snap<--

3) The disk is only flushed if there was no error. Previously it was
always flushed without checking the error.

Thanks a lot,

Best regards,
    Sougata
> 
> > Also hfsplus_sync_fs is explicitly called
> > from mount/unmount, which is unnecessary. During mount/unmount we only want
> > to set/clear volume dirty sate.



  reply	other threads:[~2014-07-18  8:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17 16:32 [PATCH 1/1] hfsplus: skip unnecessary volume header sync Sougata Santra
2014-07-17 20:59 ` Andrew Morton
2014-07-18  8:35   ` Sougata Santra [this message]
2014-07-18  9:01     ` Vyacheslav Dubeyko
2014-07-18  9:49       ` Sougata Santra
2014-07-19 10:58         ` Vyacheslav Dubeyko
2014-07-19 12:18           ` sougata santra
2014-07-18  8:28 ` Vyacheslav Dubeyko
2014-07-18  9:24   ` Sougata Santra
2014-07-19 11:23     ` Vyacheslav Dubeyko
2014-07-19 11:59       ` sougata santra

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=1405672537.25052.18.camel@ultrabook \
    --to=sougata@tuxera.com \
    --cc=akpm@linux-foundation.org \
    --cc=fabf@skynet.be \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    /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;
as well as URLs for NNTP newsgroup(s).