linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "William A. (Andy) Adamson" <androsadamson@gmail.com>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: bhalevy@panasas.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 0/2] pnfs-submit Re-initialize pnfs_layout_type when segs list is empty
Date: Tue, 13 Jul 2010 10:06:28 -0400	[thread overview]
Message-ID: <AANLkTinBNIUOzcqwFffYLgwne1KGoZOiADhjt2IHDIFm@mail.gmail.com> (raw)
In-Reply-To: <4C3C2B93.9050407@panasas.com>

On Tue, Jul 13, 2010 at 5:02 AM, Boaz Harrosh <bharrosh@panasas.com> wr=
ote:
> On 07/12/2010 09:07 PM, William A. (Andy) Adamson wrote:
>> On Mon, Jul 12, 2010 at 1:07 PM, Boaz Harrosh <bharrosh@panasas.com>=
 wrote:
>>> >
>
>>> > And is this a layoutdriver specific? It looks like generic busine=
ss only.
>>
>> The file layout driver zero's the stripe unit. I anticipate other
>> fields that would need to be zeroed.
>>
>
> The stripe_unit is per segment surly. It should be set on new segment
> received. (Again the mix up between layout_type and layout_segment)
> If stripe_unit also doubles for a flag that says, I have a layout.
> Then zero it on free_lseg, when identified as last.


When we only have full file layouts, the only two possible layout
segments are full file segments with different iomodes, and so must
have an equal stripe unit.

I can init on free_lseg - agreed.

>
> (Which also means that you are hard coding all-file-layout at heart
> =A0of files-layout-driver)
>
>>>
>>> As I said above: on_last_segment/remove-from-client-list can call a=
 specific
>>> free_lo_stateid, could be nice for readability.
>>
>> It does: I call pnfs_set_layout_stateid() with the zero_stateid. Tha=
t
>> is very clear.
>>
>> I just don't see the need for two more states.
>
> You might miss-understood me. By state I meant: "a point in code wher=
e
> it is identified that this is first/last segment" This already exists=
=2E
> I did not mean like an NFS4 state machine.
>
> I wish we had functions that said _on_first_seg, on_last_seg. But
> that's just me.
>
>> We have a point in the
>> generic code where the layout segment list is empty. Therefore the
>> pnfs_layout_type information is stale. Simply initialize and all the
>> checks we have work.
>>
>
> That is what I meant that I do not like. The initialize should be on
> first_seg_insert, Not "last_seg_remove in case of re-insert". The onl=
y
> check should be list-is-empty.

No! The layout stateid needs to be zeroed as well.

>Then on first insert, all should be initialized.

I guess we just do things differently. I for one like to clean up the
dishes after the meal, not wait until before the next meal! But that's
just me.

-->Andy

>
> But I guess it's hard to talk about this. Submit the last fixes and I
> can RFC my approach. I hope when you see it you'll be convinced.

OK.

-->Andy

>
> Thanks
> Boaz
>
>> -->Andy
>

      reply	other threads:[~2010-07-13 14:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-09 16:39 [PATCH 0/2] pnfs-submit Re-initialize pnfs_layout_type when segs list is empty andros
2010-07-09 16:39 ` [PATCH 1/2] SQUASHME pnfs-submit: reinitialize pnfs_layout_type andros
2010-07-09 16:39   ` [PATCH 2/2] SQUASHME pnfs-submit file layout free_layout init_only andros
2010-07-12 16:10   ` [PATCH 1/2] SQUASHME pnfs-submit: reinitialize pnfs_layout_type Benny Halevy
2010-07-12 15:48 ` [PATCH 0/2] pnfs-submit Re-initialize pnfs_layout_type when segs list is empty Boaz Harrosh
2010-07-12 16:36   ` William A. (Andy) Adamson
     [not found]     ` <AANLkTinjab9XPUjRe2knYyRQg8j9pmc9Gdk_fRkGGfY2-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-07-12 17:07       ` Boaz Harrosh
2010-07-12 18:07         ` William A. (Andy) Adamson
     [not found]           ` <AANLkTilSPhZ1aBtU1taI8UnKQ9CddgaK3oQ1xoSMOuh3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-07-13  9:02             ` Boaz Harrosh
2010-07-13 14:06               ` William A. (Andy) Adamson [this message]

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=AANLkTinBNIUOzcqwFffYLgwne1KGoZOiADhjt2IHDIFm@mail.gmail.com \
    --to=androsadamson@gmail.com \
    --cc=bhalevy@panasas.com \
    --cc=bharrosh@panasas.com \
    --cc=linux-nfs@vger.kernel.org \
    /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).