Re: [Cluster-devel] [PATCH][try6] VFS: new want_holesize and got_holesize buffer_head flags for fiemap

2014-10-22 Thread Christoph Hellwig
This looks like a big indicator that get_blocks is the wrong
interface for fiemap.



Re: [Cluster-devel] [PATCH][try6] VFS: new want_holesize and got_holesize buffer_head flags for fiemap

2014-10-22 Thread Steven Whitehouse

Hi,

On 22/10/14 07:04, Christoph Hellwig wrote:

This looks like a big indicator that get_blocks is the wrong
interface for fiemap.



The question is then, what a better interface would look like? We could 
have get_hole as an extra operation I suppose. Not sure that I really 
see why thats better or worse than the extra flag at the moment? or did 
you have something else in mind?


Steve.



Re: [Cluster-devel] [PATCH][try6] VFS: new want_holesize and got_holesize buffer_head flags for fiemap

2014-10-22 Thread Bob Peterson
- Original Message -
 This looks like a big indicator that get_blocks is the wrong
 interface for fiemap.

Hi Christoph,

Yes, I thought about that.
One of my early prototypes had a separate function used by fiemap.
Function __generic_block_fiemap would call get_block() which
returned an indication of a hole as it does today. When it saw
the hole, fiemap called a new function get_hole_size() that was
passed in like get_block. The problem is: it's grossly inefficient,
since the new function get_hole_size() has to redo most of the work
that get_block just did (at least in the case of GFS2). (Which in the
case of a 1PB sparse file is non-trivial, since it involves several
levels of metadata indirection). Combining it with get_block made it
much more efficient.

Making a separate get_block_map_fiemap() function just seems like an
exercise in redundancy.

Regards,

Bob Peterson
Red Hat File Systems



Re: [Cluster-devel] [PATCH 02/12] quota: Allow each filesystem to specify which quota types it supports

2014-10-22 Thread Christoph Hellwig
On Tue, Oct 21, 2014 at 04:38:26PM +0200, Jan Kara wrote:
 Currently all filesystems supporting VFS quota support user and group
 quotas. With introduction of project quotas this is going to change so
 make sure filesystem isn't called for quota type it doesn't support by
 introduction of a bitmask determining which quota types each filesystem
 supports.

Why don't you keep this bitmask in the dquot.c instead of pushing it
to the caller?  So far usage of s_dquot is mostly confined to dquot.c
(with a few leaks to the filesystems using it), so keeping it that
way seems like a good idea.



Re: [Cluster-devel] [PATCH 02/12] quota: Allow each filesystem to specify which quota types it supports

2014-10-22 Thread Jan Kara
On Wed 22-10-14 18:29:15, Christoph Hellwig wrote:
 On Tue, Oct 21, 2014 at 04:38:26PM +0200, Jan Kara wrote:
  Currently all filesystems supporting VFS quota support user and group
  quotas. With introduction of project quotas this is going to change so
  make sure filesystem isn't called for quota type it doesn't support by
  introduction of a bitmask determining which quota types each filesystem
  supports.
 
 Why don't you keep this bitmask in the dquot.c instead of pushing it
 to the caller?  So far usage of s_dquot is mostly confined to dquot.c
 (with a few leaks to the filesystems using it), so keeping it that
 way seems like a good idea.
  So there are two reasons:
1) Currently if you call quotactl() with invalid quota type you'll get
EINVAL. To maintain this with addition of project quotas you need to check
the types early before calling check_quotactl_permission() and other
checks.

2) I didn't want filesystem quotactl callbacks to deal with quota types
they don't support. Sure each fs could do a type check in the callback but
this looked easier.

Now I see your point about s_dquot and I can move allowed_types out of
s_dquot if that makes you happier. But otherwise what I did still seems as
the best solution to me.

Honza
-- 
Jan Kara j...@suse.cz
SUSE Labs, CR