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: Sat, 19 Jul 2014 15:18:20 +0300 Message-ID: <53CA620C.5060009@tuxera.com> References: <1405614762.25052.8.camel@ultrabook> <20140717135929.1a9fa7279cbb4e7a57761213@linux-foundation.org> <1405672537.25052.18.camel@ultrabook> <1405674110.2626.39.camel@slavad-CELSIUS-H720> <1405676989.25052.42.camel@ultrabook> <7632517E-83B7-4956-BB50-9A2FD1E13A7B@dubeyko.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Andrew Morton , , , Fabian Frederick To: Vyacheslav Dubeyko Return-path: Received: from nbl-ex10-fe01.nebula.fi ([188.117.32.121]:56860 "EHLO ex10.nebula.fi" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755208AbaGSMSX (ORCPT ); Sat, 19 Jul 2014 08:18:23 -0400 In-Reply-To: <7632517E-83B7-4956-BB50-9A2FD1E13A7B@dubeyko.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 07/19/2014 01:58 PM, Vyacheslav Dubeyko wrote: > > On Jul 18, 2014, at 1:49 PM, Sougata Santra wrote: > >> On Fri, 2014-07-18 at 13:01 +0400, Vyacheslav Dubeyko wrote: >>> On Fri, 2014-07-18 at 11:35 +0300, Sougata Santra wrote: >>> >>> [snip] >>>> 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. >>> >>> So, do you mean that filemap_write_and_wait() and hfsplus_submit_bio() >>> doesn't request writing on a volume? >> Yes it did, but it wrote in the page-cache ?. > > The hfsplus_submit_bio() makes bio allocation via bio_alloc() and, then, > send request on write into elevator queue by means of submit_bio_wait(). > Moreover, submit_bio_wait() submits a bio, and wait until it completes. > It means that we have changed block on volume after returning from > hfsplus_submit_bio(). AFAIK, this is not always correct, we don't change the block on the volume. The block device driver uses special command to initiate data transfer with the disk controller. Then the disk controller uses DMA to transfer the data and raises an interrupt that it is done with the data transfer. That does not mean that the data is there is in the non-volatile memory of the disk. If there is a write back cache in the disk then the data is stored there. Then the disk controller uses its own algorithm to write back data from its own cache to disk. The flush operation sends an empty bio requesting flush of the write back cache. My argument was when there was a problem of transferring the data into disk cache, why bother to flush it. And if any data has already been written to disk cache, the disk controller takes care and writes it into non volatile memory, given that there is no power-off/plug-out scenarios. > > The filemap_write_and_wait() calls do_writepages() finally. It calls > mapping->a_ops->writepages(). So, it means that do_writepages() calls > hfsplus_writepages(). As a result, finally, requests on write are placed in > elevator queue. And, again, dirty pages of inode->i_mapping (page cache) > will be on volume after returning from filemap_write_and_wait(). > > This is my understanding of hfsplus_sync_fs() method's logic. Please, > correct me if I am wrong. > > So, if you try to prevent from blkdev_issue_flush() then it doesn't mean > that you prevent from writing special files and volume header on volume. > I suppose that logic of writing in any case was special policy. Because > fsck utility can help to us in some bad situations. This is my understanding > of this method's logic. Please don't get incorrect. I am not sure if should be done or not. Obviously if it is not the best thing to do, we won't do it :-). > > Thanks, > Vyacheslav Dubeyko. > Thanks a lot, Sougata Santra