[ disclaimer - I don't have any say anyway, but this is my impression ]
> 1) My kernel patch does the minimal work it needs to, and the rest is done
> as user-space utilities, which I think is the desirable way to go, as
> we don't wan't the kernel to be able to do a mke2fs necessarily. So far
> so good?
Yep.
> 2) I re-used what existing kernel code I could, made it more generic in
> some cases, to handle what I needed to do. Presumably this is the
> correct way to go?
Indeed, unless there is a potention performance impact.
> 3) I have read about formatting of code for the kernel, and have tried to
> follow them. Would patches be rejected for having too many comments?
Probably not, unless it needed them because the code was obscure. If
/what/ the functions do is documented and /how/ they do it is obvious,
additional comments should only be necessary when non-obvious or tricky
things are happening.
The theory is that kernel hackers don't need to be told:
int i; /* this is used as a loop counter further on */
when
int i;
is enough.
> 4) There are some known "issues" in the patch (e.g. it turns off quota
> checking while not having the lock on the filesystem for a "brief instant"
> because it is calling a function which grabs the superblock lock itself).
> Is this "permitted", considering the combination of quotas, frequency of
> filesystem resize, and a "one-line" chance of being interrupted before
> getting the superblock lock again is considered small enough to justify
> not making more changes to the kernel?
Safety is better. A common pattern is something like this:
useful_func(..)
{
lock_something(&something_lock);
...
unlock_something(&something_lock);
}
which gets turned into:
useful_func(..)
{
lock_something(&something_lock);
__useful_func(..);
unlock_something(&something_lock);
}
and __useful_func(..) does the rest of the work. That way you
can be more flexible about what you have to lock for, &c.
> 5) Since the userspace tools aren't 100% ready for public consumption, is
> there any chance of getting the patch into 2.3.x? I wanted to have the
> user code ready before trying to get the patch into the mainstream kernel
> (to avoid more patches to the kernel), but in my testing the kernel code
> is ready for use. The resize functions added by the patch sits totally off
> the main code path, so do not have any real performance impact at all if
> they are added.
You'd have to ask the man himself about this.
> 6) With a new feature like this, would it be required to have an option to
> turn it on/off via menuconfig? The patch will add a useful capability
> to the kernel even if people don't have the user-space tools required for
> full functionality (ie an unmodified "mount" command can do wonders for
> you if required). However, to really get use out of this patch, you need
> to be able to resize your underlying device size via md or hardware RAID,
> or most likely LVM.
If the feature and code are self-contained, add more than a couple of k to
the kernel or have potential performance implications, and are not useful
for everyone, then it should probably be optional, IMO.
MAtthew.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/