From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [RFC] Tux3 for review Date: Sat, 21 Jun 2014 12:29:01 -0700 Message-ID: <1403378941.2177.24.camel@dabdike.int.hansenpartnership.com> References: <5376B273.7000800@partner.samsung.com> <20140518235555.GC18954@dastard> <537AA802.408@phunq.net> <20140520031802.GF18954@dastard> <20140613103216.GA4589@amd.pavel.ucw.cz> <02d3b094-808c-4b17-903d-1280d451704b@phunq.net> <20140613202039.GA23872@amd.pavel.ucw.cz> <1402932354.2197.61.camel@dabdike.int.hansenpartnership.com> <20140619082129.GA4309@xo-6d-61-c0.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: =?UTF-8?Q?Luk=C3=A1=C5=A1?= Czerner , Pavel Machek , Dave Chinner , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Linus Torvalds , Andrew Morton To: Daniel Phillips Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Thu, 2014-06-19 at 14:58 -0700, Daniel Phillips wrote: > On Thursday, June 19, 2014 2:26:48 AM PDT, Luk=E1=A8 Czerner wrote: > > Let me remind you some more important problems Dave brought up, > > including page forking: > > > > " > > The hacks around VFS and MM functionality need to have demonstrate= d > > methods for being removed. >=20 > We already removed 450 lines of core kernel workarounds from Tux3 wit= h an=20 > approach that was literally cut and pasted from one of Dave's emails.= Then=20 > Dave changed his mind. Now the Tux3 team has been assigned a research= =20 > project to improve core kernel writeback instead of simply adapting t= he=20 > approach that is already proven to work well enough. That is a rather= =20 > blatant example of "perfect is the enemy of good enough". Please read= the=20 > thread. That's a bit disingenuous: the concern has always been how page forking interacted with writeback. It's not new, it was one of the major thing= s brought up at LSF 14 months ago, so you weren't just assigned this. > > We're not going to merge that page > > forking stuff (like you were told at LSF 2013 more than a year ago= : > > http://lwn.net/Articles/548091/) without rigorous design review an= d > > a demonstration of the solutions to all the hard corner cases it > > has. The current code doesn't solve them (e.g. direct IO doesn't > > work in tux3), and there's no clear patch set we can review that > > demonstrates how it is all supposed to work. i.e. you need to > > separate out all the page forking code into a separate patchset fo= r > > review, independent of the tux3 code and applies to the core mm/ > > code. > > " >=20 > Direct IO is a spurious issue. To recap: direct IO does not introduce= any=20 > new page forking issues. All of the page forking issues already exist= with=20 > normal buffered IO and mmap. We have little interest and scant availa= ble=20 > time for heading off on a tangent to implement direct IO at this poin= t just=20 > as a precondition for merging. The specific concern is that page forking cannot be made to work with direct io. Asserting that it doesn't cause any additional problems isn't an answer to that concern. Direct IO isn't actually a huge issue for most filesystems (I mean even vfat has it). The fact that you thin= k it is such a huge deal to implement for tux3 tends to lend credence to this viewpoint. The point is that if page forking won't work with direct IO at all, the= n it's a broken design and there's no point merging it. > On the other hand, page forking itself has a number of interesting is= sues.=20 > Hirofumi is currently preparing a set of core kernel patches for revi= ew.=20 > These patches explicitly do not attempt to package page forking up in= to a=20 > nice and easy API that other filesystems could patch in tomorrow. Tha= t=20 > would be an unreasonable research burden on our small development tea= m.=20 > Instead, we show how it works in Tux3, and if other filesystems want = to get=20 > those benefits, they can make similar changes. If we (the kernel comm= unity)=20 > are lucky enough to find a pattern in it such that substantial parts = of the=20 > code can be abstracted into a library, then good. But requiring such = a=20 > library to be developed as a precondition to merging Tux3 is unreason= able. OK, can we take a step back and ask why you're so keen to push this int= o the tree? The usual reason is ease of maintenance because in-tree filesystems get updated as the vfs and mm APIs change. However, the reciprocal side of that is using standard VFS and MM APIs to make this update and maintenance easy. The reason no-one wants an in-tree filesystem that implements its own writeback by hacking into the curren= t writeback system is that it's a huge maintenance burden. Every time writeback gets tweaked, tux3 will break meaning either we double the burden on people updating writeback (to try to figure out how to replicate the change in tux3) or we just accept that tux3 gets broken. The former is unacceptable to the filesystem and mm people and the latter would mean there's not really much point merging tux3 if we just keep breaking it ... it's better to keep it out of tree where the breakages can be fixed by people who understand them on their own timescales. The object of the exercise is *not* for you to convert every filesystem to tux3, it's to see if there's a way of integrating enough of page forking into the current writeback code that tux3 uses standard APIs an= d doesn't multiply the burden on the people who maintain and update the writeback code. > > " > > Then there's all the writeback hacks. You've simply copy-n-pasted > > most of fs-writeback.c, including duplicating structures like stru= ct > > wb_writeback_work and then hacked in crap (kallsyms lookups!) to b= e > > able to access core structures from kernel module context > > (tux3_setup_writeback(), I'm looking at you). This is completely > > unacceptible for a merge. Again, you need to separate out all the > > writeback changes you need into an independent patchset so that th= ey > > can be reviewed independently of the tux3 code that uses it. > > " >=20 > That was already fixed as noted above, and all the relevant changes w= ere=20 > already posted as an independent patch set. After that, some develope= rs=20 > weighed in with half formed ideas about how the same thing could be d= one=20 > better, but without concrete suggestions. There is nothing wrong with= half=20 > formed ideas, except when they turn into a way of blocking forward=20 > progress. See "perfect is the enemy of good enough" above. Could you post the url to the new series, please, I must have missed it= ; seeing the patches that implement the API for insertion into the writeback code would certainly help frame this discussion. > It is worth noting that we (the kernel community) have been thrashing= away=20 > at the writeback problem for more than twenty years, and the current=20 > solution still leaves much to be desired. It is unfair to expect us, = the=20 > Tux3 team, to fix that mess in a week or two, just to merge our files= ystem.=20 > We prefer to adapt the existing infrastructure for now, as expressed = in the=20 > currently proposed patch set. With that, we allow core to mark our in= odes=20 > dirty just as it has always done, and we continue to use the usual in= ode=20 > writeback lists for writeback sheduling, which work just fine. So that's a misunderstanding of expectations; the actual expectation is that you won't make the writeback problem more difficult to tackle. Reimplementing writeback within your code in a way that's hacked into the system is fragile and burdensome: as I said above, it becomes doubl= e the code to maintain and tux3 breaks if its not updated. James