Re: linux-next: Tree for May 8 (dlm)

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Mon, 2013-05-13 at 12:45 -0700, Randy Dunlap wrote:
> [resending since mail server dropped it]
> 
> On 05/13/13 12:34, Randy Dunlap wrote:
> > On 05/13/13 12:31, Randy Dunlap wrote:
> >> On 05/13/13 09:30, Randy Dunlap wrote:
> >>> On 05/13/13 02:18, Steven Whitehouse wrote:
> >>>> Hi,
> >>>>
> >>>> On Thu, 2013-05-09 at 10:08 -0700, Randy Dunlap wrote:
> >>>>> On 05/09/13 09:50, David Teigland wrote:
> >>>>>> On Thu, May 09, 2013 at 09:47:45AM +1000, Stephen Rothwell wrote:
> >>>>>>> [Just forwarding to David ...]
> >>>>>>>
> >>>>>>> On Wed, 08 May 2013 11:04:45 -0700 Randy Dunlap <rdunlap@infradead.org> wrote:
> >>>>>>>>
> >>>>>>>> on x86_64:
> >>>>>>>>
> >>>>>>>> when CONFIG_GFS2_FS_LOCKING_DLM=y and CONFIG_DLM=m:
> >>>>>>>>
> >>>>>>>> fs/built-in.o: In function `gfs2_lock':
> >>>>>>>> file.c:(.text+0xa512c): undefined reference to `dlm_posix_get'
> >>>>>>>> file.c:(.text+0xa5140): undefined reference to `dlm_posix_unlock'
> >>>>>>>> file.c:(.text+0xa514a): undefined reference to `dlm_posix_lock'
> >>>>>>
> >>>>>> gfs2/file.c calls the dlm directly, so I suppose gfs2 itself needs
> >>>>>> to depend on the dlm.  It's been like this for a long time, so I
> >>>>>> don't know why it only appeared now.
> >>>>>
> >>>>> Agreed to both statements.
> >>>>>
> >>>>>>>> fs/built-in.o: In function `gdlm_cancel':
> >>>>>>>> lock_dlm.c:(.text+0xb3f57): undefined reference to `dlm_unlock'
> >>>>>>>> fs/built-in.o: In function `gdlm_unmount':
> >>>>>>>> lock_dlm.c:(.text+0xb40ff): undefined reference to `dlm_release_lockspace'
> >>>>>>>> fs/built-in.o: In function `sync_unlock.isra.4':
> >>>>>>>> lock_dlm.c:(.text+0xb420d): undefined reference to `dlm_unlock'
> >>>>>>>> fs/built-in.o: In function `sync_lock.isra.5':
> >>>>>>>> lock_dlm.c:(.text+0xb42d9): undefined reference to `dlm_lock'
> >>>>>>>> fs/built-in.o: In function `gdlm_put_lock':
> >>>>>>>> lock_dlm.c:(.text+0xb45e7): undefined reference to `dlm_unlock'
> >>>>>>>> fs/built-in.o: In function `gdlm_mount':
> >>>>>>>> lock_dlm.c:(.text+0xb4928): undefined reference to `dlm_new_lockspace'
> >>>>>>>> lock_dlm.c:(.text+0xb4c75): undefined reference to `dlm_release_lockspace'
> >>>>>>>> fs/built-in.o: In function `gdlm_lock':
> >>>>>>>> lock_dlm.c:(.text+0xb529f): undefined reference to `dlm_lock'
> >>>>>>
> >>>>>> lock_dlm.c is GFS2_FS_LOCKING_DLM which depends on DLM.
> >>>>>> Is that not correct?
> >>>>>
> >>>>> The problem is that GFS2_FS_LOCKING_DLM is a bool.  It depends on DLM,
> >>>>> which is a tristate with a value of 'm', so the bool is true (as long
> >>>>> as DLM != 'n').
> >>>>>
> >>>>> One option is to make GFS2_FS_LOCKING_DLM depend on "DLM != n", but a
> >>>>> better fix is to make GFS2_FS depend on DLM, like you said above.
> >>>>>
> >>>>>
> >>>>
> >>>> Does this look correct? As Dave says this has not changed for some time.
> >>>> It seems that every time we try to get this right, there is always some
> >>>> corner case that is missed :( 
> >>>
> >>> Sorry, I misspoke above.   It will have to depend on DLM=y since DLM=m
> >>> is what is causing these build errors.
> >>
> >> and that is too strict.  It needs to allow for both dlm and gfs2 built as
> >> loadable modules.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> >>>> We can't make GFS2_FS depend on DLM as otherwise there would be no
> >>>> reason to have GFS2_FS_LOCKING_DLM, at least if I've understood the
> >>>> issue here correctly. So I've come up with the following... does it look
> >>>> ok?
> >>>>
> >>>>
> >>>> diff --git a/fs/gfs2/Kconfig b/fs/gfs2/Kconfig
> >>>> index eb08c9e..edbad96 100644
> >>>> --- a/fs/gfs2/Kconfig
> >>>> +++ b/fs/gfs2/Kconfig
> >>>> @@ -26,7 +26,7 @@ config GFS2_FS
> >>>>  config GFS2_FS_LOCKING_DLM
> >>>>  	bool "GFS2 DLM locking"
> >>>>  	depends on (GFS2_FS!=n) && NET && INET && (IPV6 || IPV6=n) && \
> >>>> -		HOTPLUG && DLM && CONFIGFS_FS && SYSFS
> >>>> +		HOTPLUG && (DLM!=n) && CONFIGFS_FS && SYSFS
> >>>
> >>> 		HOTPLUG && DLM=y && CONFIGFS_FS && SYSFS
> >>
> >> 		HOTPLUG && CONFIGFS_FS && SYSFS && (DLM=y || DLM=GFS2_FS)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> >> I think.
> > 
> > tested and works AFAICT.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
Yes, that looks better to me. Can you send that as a patch? Then I can
stick it in the tree for further testing. Thanks,

Steve.