DragonFlyBSD

Francois,

as I said previously on IRC I disagree with this approach to statfs and
hence to your first two patches. You should be using the statfs entry
point and not inspect the statfs structure in the mount point.

For that, you need to simply fix the right thing, which would be the
VOP_STATFS entry of nullfs and make it populate the fields you need as
you expect. On IRC I outlined this in much more detail, so please take a
look at your logs for more info.

That the stat field in the mount point is not populated as you would
like it is not a bug and doesn't really need fixing, you should be
using the right approach instead, one that is using the API instead of
raw accesses.

Regarding the other patch, please make sure that you don't break the
initrd infrastructure in the process. Haven't looked in detail at your
patch, but it looks relatively dangerous in assuming where the root
mount is in the mount list.

Kind Regards,
Alex

On 28/08/11 11:22, Francois Tigeot wrote:

Hi,

While working on the vfs-quota branch, I found some problems in the existing
kernel VFS code.

The attached patches correspond to local commits I have created to fix them.
I'm not too sure if what I've done is ideal, and I'd like these patches
to be reviewed before pushing them to master (or not).

They have been applied to two of my machines (one desktop, one database
server), and do not seem to cause any problem.

Thanks for the remarks Alex,

Alex Hornung wrote:

as I said previously on IRC I disagree with this approach to statfs and
hence to your first two patches. You should be using the statfs entry
point and not inspect the statfs structure in the mount point.

For that, you need to simply fix the right thing, which would be the
VOP_STATFS entry of nullfs and make it populate the fields you need as
you expect.

To be honest, I had forgotten about this conversation; I'll try to
replace the direct struct mount accesses by the use of VOP_STATFS.

That the stat field in the mount point is not populated as you would
like it is not a bug and doesn't really need fixing, you should be
using the right approach instead, one that is using the API instead of
raw accesses.

When I tried to use the STATFS function API, it didn't work as expected
and caused kernel panics.
I now see that this function was not completely implemented for tmpfs.

What is the rationale for not using the statfs information present in
struct mount ? It was in a very convenient location...

Regarding the other patch, please make sure that you don't break the
initrd infrastructure in the process. Haven't looked in detail at your
patch, but it looks relatively dangerous in assuming where the root
mount is in the mount list.

This is the part where I'm the less sure of what I should be doing. The
code was copied verbatim from the start_init() function; I assumed it
was correct in the first place.
I didn't remove it from there; when I tried to I once again got problems
with an unitialized namecache layer, but this time later in the boot
process.

Best Regards,

While working on the vfs-quota branch, I found some problems in the existing
kernel VFS code.

The attached patches correspond to local commits I have created to fix them.
I'm not too sure if what I've done is ideal, and I'd like these patches
to be reviewed before pushing them to master (or not).

Francois Tigeot wrote:

Alex Hornung wrote:

as I said previously on IRC I disagree with this approach to statfs and
hence to your first two patches. You should be using the statfs entry
point and not inspect the statfs structure in the mount point.

For that, you need to simply fix the right thing, which would be the
VOP_STATFS entry of nullfs and make it populate the fields you need as
you expect.

To be honest, I had forgotten about this conversation; I'll try to
replace the direct struct mount accesses by the use of VOP_STATFS.

I'm focusing on the first patch for now, the one which initializes
mp->mnt_stat.f_mntonname from nullfs_mount()

That the stat field in the mount point is not populated as you would
like it is not a bug and doesn't really need fixing, you should be
using the right approach instead, one that is using the API instead of
raw accesses.

Guess where VOP_STATFS takes the values it returns ? From mp->mnt_stat :
http://opengrok.wolfpond.org/source/xref/sys/vfs/nullfs/null_vfsops.c#nullfs_statfs

nullfs_statfs is a very simple function; it only copies existing values.

The initial name of the mount point can only be known from the
nullfs_mount function; if this patch is not applied, VOP_STATFS will
never be able to return the correct values.

I'm afraid patch-0008 has to be applied.

On Thu, Sep 01, 2011 at 11:38:42PM +0200, Francois Tigeot wrote:

The initial name of the mount point can only be known from the
nullfs_mount function; if this patch is not applied, VOP_STATFS will
never be able to return the correct values.

New patch attached.