Re: 2.6.22-rc6-mm1: UNION_FS=y + BLOCK=n = compile error

Erez Zadok
2007-07-08T12:02:25Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2808 bytes --]

I'm adding Jens Axboe to the CC list (BLOCK layer maintainer).

In message <20070707020142.GA3492@stusta.de>, Adrian Bunk writes:
> On Thu, Jun 28, 2007 at 03:43:21AM -0700, Andrew Morton wrote:
> >...
> > Changes since 2.6.22-rc4-mm2:
> >...
> > git-unionfs.patch
> >...
> > git trees
> >...
>
> CONFIG_UNION_FS=y, CONFIG_BLOCK=n results in the following compile error:
>
> <-- snip -->
>
> ...
> CC fs/unionfs/file.o
> /home/bunk/linux/kernel-2.6/linux-2.6.22-rc6-mm1/fs/unionfs/file.c:147: error: ‘file_fsync’ undeclared here (not in a function)
> make[3]: *** [fs/unionfs/file.o] Error 1
> ...
>
> <-- snip -->
>
> cu
> Adrian

Adrian, thanks. I was able to reproduce this compiler error. But I think
there may be a problem with the mainline kernel itself.

Unionfs defines its fsync file op to file_fsync. file_fsync, defined in
fs/sync.c, appears to be a generic syncing function that can be used by
other file systems. In fact, the comment above it says "Generic function to
fsync a file." From fs/Makefile, it looks like sync.c gets compiled
regardless whether CONFIG_BLOCK is defined or not.

Moreover, file_fsync() makes a call to sync_blockdev(), clearly a function
which depends on CONFIG_BLOCK=y. However, include/linux/buffer_head.h
defines sync_blockdev() to a noop if CONFIG_BLOCK=n:

static inline int sync_blockdev(struct block_device *bdev) { return 0; }

This further indicates to me that someone went through the effort of making
file_fsync available and operational *even* if CONFIG_BLOCK=n.

But here's the problem: if CONFIG_BLOCK=n, the function file_fsync exists
and gets compiled into the kernel, but the extern definition for it is not
compiled in (it's wrapped in an #ifdef CONFIG_BLOCK in buffer_head.h). This
looks to me like a discrepancy: why compile the file_fsync function, and
even ensure that it will find a dummy sync_blockdev() symbol, if one doesn't
provide an extern for it?

One more oddity: the EXPORT_SYMBOL for fs/sync.c:file_fsync() is in
fs/buffer.c, and buffer.c does not get compiled if CONFIG_BLOCK=n. I
thought all EXPORT_SYMBOL calls are supposed to be near their respective
functions, no?

I suggest one of two things:

1. If you agree with me as to this discrepancy, then we can move the extern
for file_fsync() outside the #ifdef CONFIG_BLOCK in buffer_head.h -- it
should be defined regardless. Then move the EXPORT_SYMBOL(file_fsync)
from buffer.c to sync. I can provide a small patch right away.

2. If you disagree, then I can change Unionfs to avoid calling file_fsync
directly. I can define a unionfs_fsync file method which does the usual
stuff (pass the op to the layer below).

My preference is option #1; I'd like to avoid defining any more stackable
wrappers than I have to.

Thanks,
Erez.