ocfs2: Let ocfs2_setattr use new truncate sequence.
in [Kernel]
|
Prev: Question on siig sata 3 controller
Next: [PATCH] of/device: Move struct of_device define outside of CONFIG_OF_DEVICE test
From: Christoph Hellwig on 11 Jun 2010 03:00 This looks correct, but still has the second if ATTR_SIZE block that I commented on last time. I'd really prefer if the filesystems could move the truncate handling into a single conditional to simplify auditing for it and possibly splitting it out into a separate method later. And btw, the S_ISREG check which you only have on the first ATTR_SIZE check is superflous, the VFS only does ATTR_SIZE calls on regular files. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tao Ma on 11 Jun 2010 03:20 Hi Christoph, On 06/11/2010 02:56 PM, Christoph Hellwig wrote: > This looks correct, but still has the second if ATTR_SIZE block that > I commented on last time. I'd really prefer if the filesystems could > move the truncate handling into a single conditional to simplify > auditing for it and possibly splitting it out into a separate method > later. oh, that would be much work for ocfs2 to do from my perspective. So I would really want to leave it as-is and I have add it to my to-do list. > > And btw, the S_ISREG check which you only have on the first ATTR_SIZE > check is superflous, the VFS only does ATTR_SIZE calls on regular files. yeah, I can remove it. Regards, Tao -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 11 Jun 2010 03:30 On Fri, Jun 11, 2010 at 03:16:13PM +0800, Tao Ma wrote: > Hi Christoph, > > On 06/11/2010 02:56 PM, Christoph Hellwig wrote: > >This looks correct, but still has the second if ATTR_SIZE block that > >I commented on last time. I'd really prefer if the filesystems could > >move the truncate handling into a single conditional to simplify > >auditing for it and possibly splitting it out into a separate method > >later. > oh, that would be much work for ocfs2 to do from my perspective. So I > would really want to leave it as-is and I have add it to my to-do list. Oh right, you start a new transaction there. Sorry, ignore my request and keep it as it is for now. I don't think doing truncatate in separate transactions actually is correct, though but that's no change with this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joel Becker on 11 Jun 2010 04:00 On Fri, Jun 11, 2010 at 12:50:20AM -0700, Joel Becker wrote: > On Fri, Jun 11, 2010 at 09:19:45AM +0200, Christoph Hellwig wrote: > > On Fri, Jun 11, 2010 at 03:16:13PM +0800, Tao Ma wrote: > > > oh, that would be much work for ocfs2 to do from my perspective. So I > > > would really want to leave it as-is and I have add it to my to-do list. > > > > Oh right, you start a new transaction there. Sorry, ignore my request > > and keep it as it is for now. I don't think doing truncatate in separate > > transactions actually is correct, though but that's no change with this > > patch. > > Christoph, > You're missing the part where actual truncate (reduce i_size) > sets i_size in ocfs2_truncate_file(). So this later code doesn't get > triggered for the truncate case. It exists for the extend case, where > we extend the allocation in multiple clean transactions, then finally > set i_size in a final transaction. Actually, ocfs2_extend_file() appears to handle it too. I don't think it used to - there were places that had issues with commit_write()'s update of i_size, etc. But that's ancient history. I wonder if Mark knows. joel -- Dort wo man B�cher verbrennt, verbrennt man am Ende auch Mensch. (Wherever they burn books, they will also end up burning people.) - Heinrich Heine Joel Becker Principal Software Developer Oracle E-mail: joel.becker(a)oracle.com Phone: (650) 506-8127 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joel Becker on 11 Jun 2010 04:00
On Fri, Jun 11, 2010 at 09:19:45AM +0200, Christoph Hellwig wrote: > On Fri, Jun 11, 2010 at 03:16:13PM +0800, Tao Ma wrote: > > oh, that would be much work for ocfs2 to do from my perspective. So I > > would really want to leave it as-is and I have add it to my to-do list. > > Oh right, you start a new transaction there. Sorry, ignore my request > and keep it as it is for now. I don't think doing truncatate in separate > transactions actually is correct, though but that's no change with this > patch. Christoph, You're missing the part where actual truncate (reduce i_size) sets i_size in ocfs2_truncate_file(). So this later code doesn't get triggered for the truncate case. It exists for the extend case, where we extend the allocation in multiple clean transactions, then finally set i_size in a final transaction. Joel -- "When choosing between two evils, I always like to try the one I've never tried before." - Mae West Joel Becker Principal Software Developer Oracle E-mail: joel.becker(a)oracle.com Phone: (650) 506-8127 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |