Messages in this thread | | | Subject | Re: [PATCH 01/16] GFS2: Core header files | From | Steven Whitehouse <> | Date | Thu, 31 Aug 2006 16:07:20 +0100 |
| |
Hi,
Thanks for the comments...
On Thu, 2006-08-31 at 16:16 +0200, Jan Engelhardt wrote: > >+ * > >+ * This copyrighted material is made available to anyone wishing to use, > >+ * modify, copy, or redistribute it subject to the terms and conditions > >+ * of the GNU General Public License v.2. > >+ */ > > "v2" perhaps? From a math pov, the extra dot implies v0.2. > or you could argue that the addition of a space (i.e. v. 2) would be correct since its an abbreviation, but point taken and I'll clean it up shortly.
> >+struct gfs2_log_operations; > > I would suggest listing only struct lines that are actually required, i.e. the > compiler would barf without them. > Ok. I'll take a look and see which of them are not required.
> >+enum { > >+ /* Actions */ > >+ HIF_MUTEX = 0, > >+ HIF_PROMOTE = 1, > >+ HIF_DEMOTE = 2, > >+ HIF_GREEDY = 3, > > I leave it to you whether going with the above or > > enum { > HIF_MUTEX = 0, > HIF_PROMOTE, > HIF_DEMOTE, > HIF_GREEDY, > ... > }; > > If these values need to stay the same, for example to maintain on-disk > compatibility, I prefer the former, though. > They are not on-disk visible (they'd in in gfs2_ondisk.h otherwise) but we may well want to change them as I think it should be possible to merge the gh_flags and gh_iflags fields in the struct gfs2_holder to save a bit of space.
> >+ /* Quota stuff */ > >+ > >+ struct gfs2_quota_data *al_qd[4]; > > What four quotas can there be? Use the MAXQUOTAS macro if feasible. > Its related to the way the GFS2's fuzzy quota system works. In order to avoid contention on a global quota structure, each node has its own which is synced back to the master one from time to time. This is done according to user specified constants and more frequently in the case of users nearing their quota. In the default case this means that no user can exceed their quota by more than twice, in practice a user would have to work very hard to manage even that.
As a result there are the overall user/group limits and the local differences which are saved to by synced back to the master quota information, hence four of them.
> >+struct gfs2_quota_lvb { > >+ uint32_t qb_magic; > >+ uint32_t __pad; > >+ uint64_t qb_limit; /* Hard limit of # blocks to alloc */ > >+ uint64_t qb_warn; /* Warn user when alloc is above this # */ > >+ int64_t qb_value; /* Current # blocks allocated */ > >+}; > > Is this an on-disk structure or why is there a __pad field? > > > > Jan Engelhardt
It isn't an on-disk structure, on the other hand we treat it as one. It probably ought to be changed to use __be types. Its the structure which the quota code keeps in the lock value block of DLM locks, so it is directly shared between nodes, hence the need to treat it the same as the on-disk structures in terms of making any changes to it, and keeping it aligned, and of defined byte order etc,
Steve.
- 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/
| |