From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sougata Santra Subject: Re: [PATCH 1/1] hfsplus: skip unnecessary volume header sync Date: Fri, 18 Jul 2014 11:35:37 +0300 Message-ID: <1405672537.25052.18.camel@ultrabook> References: <1405614762.25052.8.camel@ultrabook> <20140717135929.1a9fa7279cbb4e7a57761213@linux-foundation.org> Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: , , Vyacheslav Dubeyko , Fabian Frederick To: Andrew Morton Return-path: Received: from nbl-ex10-fe01.nebula.fi ([188.117.32.121]:21719 "EHLO ex10.nebula.fi" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1760599AbaGRIfl (ORCPT ); Fri, 18 Jul 2014 04:35:41 -0400 In-Reply-To: <20140717135929.1a9fa7279cbb4e7a57761213@linux-foundation.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, 2014-07-17 at 13:59 -0700, Andrew Morton wrote: > On Thu, 17 Jul 2014 19:32:42 +0300 Sougata Santra 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.