lkml.org 
[lkml]   [2002]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Loop devices under NTFS
On Tue, Aug 27, 2002 at 04:42:56PM -0700, Adam J. Richter wrote:
> >On Tue, Aug 27, 2002 at 06:53:19AM -0700, Adam J. Richter wrote:
> >> Why?
> >>
> >> According to linux-2.5.31/Documentation/Locking,
> >> "->prepare_write(), ->commit_write(), ->sync_page() and ->readpage()
> >> may be called from the request handler (/dev/loop)."
>
> >Just because it's present in current code it doesn't mean it's right.
> >Calling aops directly from generic code is a layering violation and
> >it will not survive 2.5.
>
> Only according your own proclamation. You are arguing
> circular logic, and I am aruging a concrete benefit: we can avoid an
> extra copying of all data in the input and output paths going through
> an encrypted device.

I tell you that the address_spaces are an _optional_ abstraction. Thus
using the directly from generic code is a layering violation. This layer
of indirection was added intentionally in 2.3, and if you want to get rid
of it again please submit a patch to Al to merge the aops back into the
inode_operations vector. Otherwise I will cleanup the last remaining
violation of that layering rule in 2.5.

> While I don't have a benchmark to show you (and
> the burden of proof is upon you since you want a change), an extra
> copying of all data going through a potentially high throughput
> service (like, say, all of your file systems if you're running an
> encryptd disk), is likely to have a substantial performance impact.
> There is a real world benefit at stake here of making strong
> encryption as "cheap" to use as as possible.

I am currently reviewing a patch from Jari Ruusu that does not only
get rid of the layering violation but also provides certain advantags
for the loop-AES crypto addon he wrote/maintains. I doubt he would do
so it it kills performance for his application. Neverless I must say
I don't care at all for performace of encrypted loop: It's not merged,
and mainline correctness has a _much_ higher priority for me than
performance of external code.

You argue for performace at the cost of correctness.

> >Separating a stackalbe encryption block device from the loop driver is
> >a good idea. The current loop code is a horrible mess because it tries
> >to do the job of three drivers in one.
>
> Just saying "good idea" is no substitute for an argument about
> real world benefits, like performance, code footprint, etc.

Correctness and cleanness.

> >No, tmpfs also does not use generic_file_read but a sligh variation,
> >calling do_generic_file_read on tmpfs inodes will not always works as
> >expected. Don't do it.
>
> Your first sentence is not a clear reason why tmpfs cannot
> provide {prepare,commit}_write, and your second sentence ("Don't do
> it.") is not a reason.

It could provide them just for the sake of loop.c's layering violation
to exist for a longer time. Due to it's abuse of do_generic_file read
it would continue to have another problem.

> I have to say I haven't see much documentation of
> address_space_operations aside from the code, and a few pages about
> the page cache in _Understanding The Linux Kernel_. However, if you
> believe that loop.c is relying on some guarantee that aops does not
> officially provide but all of its implementations currently abide by,
> then simply documenting that guarantee as "official" would result in a
> kerenl that is lives within its guarantees and yet has faster performance
> for software encrypted block devices.

If you think that the guarantee that every filesystem should be pagecache
backed is worth documenting (and adopting everything to it), feel free to
submit a patch for review. I have stated above why it's not a good idea.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:28    [W:0.116 / U:2.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site