From: Peng Tao <bergwolf@gmail.com>
To: Benny Halevy <bhalevy@tonian.com>,
"Myklebust, Trond" <Trond.Myklebust@netapp.com>
Cc: Jim Rees <rees@umich.edu>,
linux-nfs@vger.kernel.org, peter honeyman <honey@citi.umich.edu>
Subject: Re: [PATCH v2 07/25] pnfsblock: add blocklayout Kconfig option, Makefile, and stubs
Date: Wed, 27 Jul 2011 01:18:28 +0800 [thread overview]
Message-ID: <CA+a=Yy63PqB4yn4sb7iTc8hb7k_Vk81zX0MjbsotMQRiN0eN-g@mail.gmail.com> (raw)
In-Reply-To: <4E2DB56A.9010908@tonian.com>
Hi, Trond and Benny,
On Tue, Jul 26, 2011 at 2:26 AM, Benny Halevy <bhalevy@tonian.com> wrote:
> On 2011-07-25 13:25, Myklebust, Trond wrote:
>>> -----Original Message-----
>>> From: Benny Halevy [mailto:bhalevy@tonian.com]
>>> Sent: Monday, July 25, 2011 10:50 AM
>>> To: Myklebust, Trond
>>> Cc: Jim Rees; linux-nfs@vger.kernel.org; peter honeyman
>>> Subject: Re: [PATCH v2 07/25] pnfsblock: add blocklayout Kconfig
>>> option, Makefile, and stubs
>>>
>>> On 2011-07-25 10:38, Myklebust, Trond wrote:
>>>>> -----Original Message-----
>>>>> From: Benny Halevy [mailto:bhalevy@tonian.com]
>>>>> Sent: Monday, July 25, 2011 10:31 AM
>>>>> To: Jim Rees
>>>>> Cc: Myklebust, Trond; linux-nfs@vger.kernel.org; peter honeyman
>>>>> Subject: Re: [PATCH v2 07/25] pnfsblock: add blocklayout Kconfig
>>>>> option, Makefile, and stubs
>>>>>
>>>>> On 2011-07-21 15:34, Jim Rees wrote:
>>>>>> From: Fred Isaman <iisaman@citi.umich.edu>
>>>>>>
>>>>>> Define a configuration variable to enable/disable compilation of
>>> the
>>>>>> block driver code.
>>>>>>
>>>>>> Add the minimal structure for a pnfs block layout driver, and
>> empty
>>>>>> list-heads that will hold the extent data
>>>>>>
>>>>>> [pnfsblock: make NFS_V4_1 select PNFS_BLOCK]
>>>>>> Signed-off-by: Peng Tao <peng_tao@emc.com>
>>>>>> Signed-off-by: Fred Isaman <iisaman@citi.umich.edu>
>>>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>>>>> [pnfs-block: fix CONFIG_PNFS_BLOCK dependencies]
>>>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>>>>> Signed-off-by: Benny Halevy <benny@tonian.com>
>>>>>> [pnfsblock: SQUASHME: port block layout code]
>>>>>> Signed-off-by: Peng Tao <peng_tao@emc.com>
>>>>>> [pnfsblock: SQUASHME: adjust to API change]
>>>>>> Signed-off-by: Fred Isaman <iisaman@citi.umich.edu>
>>>>>> [pnfs: move pnfs_layout_type inline in nfs_inode]
>>>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>>>>> [blocklayout: encode_layoutcommit implementation]
>>>>>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>>>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>>>>> Signed-off-by: Benny Halevy <benny@tonian.com>
>>>>>> [pnfsblock: layout alloc and free]
>>>>>> Signed-off-by: Fred Isaman <iisaman@citi.umich.edu>
>>>>>> [pnfs: move pnfs_layout_type inline in nfs_inode]
>>>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>>>>> Signed-off-by: Benny Halevy <benny@tonian.com>
>>>>>> [pnfsblock: define module alias]
>>>>>> Signed-off-by: Peng Tao <peng_tao@emc.com>
>>>>>> ---
>>>>>> fs/nfs/Kconfig | 8 ++-
>>>>>> fs/nfs/Makefile | 1 +
>>>>>> fs/nfs/blocklayout/Makefile | 5 +
>>>>>> fs/nfs/blocklayout/blocklayout.c | 175
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>> fs/nfs/blocklayout/blocklayout.h | 91 ++++++++++++++++++++
>>>>>> 5 files changed, 279 insertions(+), 1 deletions(-)
>>>>>> create mode 100644 fs/nfs/blocklayout/Makefile
>>>>>> create mode 100644 fs/nfs/blocklayout/blocklayout.c
>>>>>> create mode 100644 fs/nfs/blocklayout/blocklayout.h
>>>>>>
>>>>>> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
>>>>>> index 2cde5d9..be02077 100644
>>>>>> --- a/fs/nfs/Kconfig
>>>>>> +++ b/fs/nfs/Kconfig
>>>>>> @@ -79,15 +79,21 @@ config NFS_V4_1
>>>>>> depends on NFS_FS && NFS_V4 && EXPERIMENTAL
>>>>>> select SUNRPC_BACKCHANNEL
>>>>>> select PNFS_FILE_LAYOUT
>>>>>> + select PNFS_BLOCK
>>>>>> + select MD
>>>>>> + select BLK_DEV_DM
>>>>>
>>>>> Why is PNFS_BLOCK enabled automatically in all cases?
>>>>> That renders the use of modules for layout drivers totally useless.
>>>>> I sort of understand that for PNFS_FILE_LAYOUT (when my
>>>>> arm is twisted really hard behind my back :) since it
>>>>> is an integral part of RFC5661 but what's the justification
>>>>> for PNFS_BLOCK? and why blocks and not objects?
>>>>
>>>> The question is rather why did objects add a selectable compile
>>> option?
>>>
>>> Just good citizenship :)
>>>
>>>> What is the point of not compiling a given layout driver if all the
>>>> dependencies are met?
>>>
>>> Reducing build times...
>>> Building a smaller kernel when modules are disabled...
>>
>>
>> You can add a line with
>> depends on m
>>
>> to ensure that it is always compiled as a module. I think that might be
>> a good thing until we have nailed down all the issues with pNFS.
>>
>
> I'd rather leave it as is so it's easier to test without CONFIG_MODULES.
>
>>> We're fine in terms of memory consumption when CONFIG_MODULES=y since
>>> the
>>> layout driver is loaded on demand but shouldn't be worried about
>>> the other case?
>>>
>>>>
>>>> IOW: The only thing I'd change above is the select MD and select
>>>> BLK_DEV_DM: I'd prefer something like
>>>>
>>>> config PNFS_BLOCK
>>>> depends on NFS_V4_1 && MD && BLK_DEV_DM
>>>> default y
>>>
>>> This is closer to the original version.
>>> However, selecting MD and BLK_DEV_DM was proven useful to
>> automatically
>>> take
>>> care of the module dependencies without having to dive into details.
>>
>> Yes, but since the MD is a completely different layer that is not under
>> our control (well, OK, Neil is still an NFS maintainer and an MD
>> maintainer) then I'd prefer to leave it as a dependency.
>>
>> We can always add something like
>>
>> comment
>> depends on NFS_V4_1 && !BLK_DEV_DM
>> Please enable BLK_DEV_MD if you wish to enable the pNFS block
>> driver.
>
> I never new you can enable comments conditionally this way...
> It looks ok to me, I'll try it out and see how it shows in make *config
I tried the above and see some issue with this approach.
Because BLK_DEV_DM is not bool, the test !BLK_DEV_DM will not work as we wanted.
With the two lines:
comment "Please enable BLK_DEV_MD if you wish to enable the pNFS block"
depends on NFS_V4_1 && !BLK_DEV_DM
The comment will always show up there...
>
> Benny
>
>>
>>
>> Cheers
>> Trond
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Thanks,
-Bergwolf
next prev parent reply other threads:[~2011-07-26 17:18 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-21 19:34 [PATCH v2 00/25] add block layout driver to pnfs client Jim Rees
2011-07-21 19:34 ` [PATCH v2 01/25] pnfs: GETDEVICELIST Jim Rees
2011-07-21 19:34 ` [PATCH v2 02/25] pnfs: add set-clear layoutdriver interface Jim Rees
2011-07-21 19:34 ` [PATCH v2 03/25] NFS41: Let layoutcommit handle multiple segments Jim Rees
2011-07-21 19:34 ` [PATCH v2 04/25] NFS41: save layoutcommit cred after first successful layoutget Jim Rees
2011-07-21 19:34 ` [PATCH v2 05/25] pnfs: ask for layout_blksize and save it in nfs_server Jim Rees
2011-07-25 14:24 ` Benny Halevy
2011-07-21 19:34 ` [PATCH v2 06/25] pnfs: cleanup_layoutcommit Jim Rees
2011-07-25 14:26 ` Benny Halevy
2011-07-21 19:34 ` [PATCH v2 07/25] pnfsblock: add blocklayout Kconfig option, Makefile, and stubs Jim Rees
2011-07-25 14:30 ` Benny Halevy
2011-07-25 14:38 ` Myklebust, Trond
2011-07-25 14:50 ` Benny Halevy
2011-07-25 17:25 ` Myklebust, Trond
2011-07-25 18:26 ` Benny Halevy
2011-07-26 17:18 ` Peng Tao [this message]
2011-07-21 19:34 ` [PATCH v2 08/25] pnfsblock: basic extent code Jim Rees
2011-07-21 19:34 ` [PATCH v2 09/25] pnfsblock: add device operations Jim Rees
2011-07-21 19:34 ` [PATCH v2 10/25] pnfsblock: remove " Jim Rees
2011-07-21 19:34 ` [PATCH v2 11/25] pnfsblock: lseg alloc and free Jim Rees
2011-07-25 14:43 ` Benny Halevy
2011-07-21 19:34 ` [PATCH v2 12/25] pnfsblock: merge extents Jim Rees
2011-07-21 19:34 ` [PATCH v2 13/25] pnfsblock: call and parse getdevicelist Jim Rees
2011-07-21 19:34 ` [PATCH v2 14/25] pnfsblock: xdr decode pnfs_block_layout4 Jim Rees
2011-07-21 19:34 ` [PATCH v2 15/25] pnfsblock: bl_find_get_extent Jim Rees
2011-07-21 19:34 ` [PATCH v2 16/25] pnfsblock: add extent manipulation functions Jim Rees
2011-07-21 19:34 ` [PATCH v2 17/25] pnfsblock: merge rw extents Jim Rees
2011-07-21 19:34 ` [PATCH v2 18/25] pnfsblock: encode_layoutcommit Jim Rees
2011-07-21 19:34 ` [PATCH v2 19/25] pnfsblock: cleanup_layoutcommit Jim Rees
2011-07-21 19:34 ` [PATCH v2 20/25] pnfsblock: bl_read_pagelist Jim Rees
2011-07-21 19:34 ` [PATCH v2 21/25] pnfsblock: bl_write_pagelist Jim Rees
2011-07-21 19:34 ` [PATCH v2 22/25] pnfsblock: note written INVAL areas for layoutcommit Jim Rees
2011-07-21 19:34 ` [PATCH v2 23/25] pnfsblock: use pageio_ops api Jim Rees
2011-07-21 19:34 ` [PATCH v2 24/25] pnfsblock: write_pagelist handle zero invalid extents Jim Rees
2011-07-21 19:34 ` [PATCH v2 25/25] NFS41: Drop lseg ref before fallthru to MDS Jim Rees
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+a=Yy63PqB4yn4sb7iTc8hb7k_Vk81zX0MjbsotMQRiN0eN-g@mail.gmail.com' \
--to=bergwolf@gmail.com \
--cc=Trond.Myklebust@netapp.com \
--cc=bhalevy@tonian.com \
--cc=honey@citi.umich.edu \
--cc=linux-nfs@vger.kernel.org \
--cc=rees@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).