LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Konrad Rzeszutek <konrad@darnok.org>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>,
	michaelc@cs.wisc.edu, dwm@enoyolf.org, darnok@68k.org,
	pjones@redhat.com, konradr@redhat.com,
	konradr@linux.vnet.ibm.com, randy.dunlap@oracle.com,
	hpa@zytor.com, lenb@kernel.org, mike.anderson@us.ibm.com,
	dwm@austin.ibm.com, arjan@infradead.org,
	Andy Whitcroft <apw@shadowen.org>
Subject: Re: [PATCH] Add iSCSI iBFT support (v0.4.6)
Date: Fri, 8 Feb 2008 15:37:17 -0500	[thread overview]
Message-ID: <200802081537.33767.konrad@darnok.org> (raw)
In-Reply-To: <1201911489.3134.79.camel@localhost.localdomain>

On Friday 01 February 2008 19:18:09 James Bottomley wrote:
> On Wed, 2008-01-30 at 17:37 -0400, Konrad Rzeszutek wrote:
> > This patch (v0.4.6) adds

> Some pieces of the patch are obviously wrong:  find_ibft() shouldn't be
> in ibft_init ... if ibft_phys was zero, it means the bootmem reservation
> wasn't done and you shouldn't be poking about in memory which has likely
> now been overwritten.
Fixed.
>
> Also, why is ibft_phys the global variable you pass?  You never actually
> want to use the physical address, what you always end up using is the
> kernel virtual address.
>
> I'd simply use the ibft variable to point to the virtual address of the
> ibft or null if not found, then you can throw out all the phys_to_virt()
> calls.
Fixed.
>
> Also, move the reserve_bootmem into the ibft_find routines and ensure
> they're only called once on boot.  Refuse to attach the ibft driver if
> the virtual pointer ibft is null.

James,

Thanks for your review. If you wouldn't mind, can you take a look
at the version 0.4.7 (http://lkml.org/lkml/2008/2/8/350) which has your 
suggestions incorporated.

Thanks again.

      parent reply	other threads:[~2008-02-08 20:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-30 21:37 Konrad Rzeszutek
2008-02-02  0:18 ` James Bottomley
2008-02-02  0:44   ` Andrew Morton
2008-02-08 20:37   ` Konrad Rzeszutek [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=200802081537.33767.konrad@darnok.org \
    --to=konrad@darnok.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@shadowen.org \
    --cc=arjan@infradead.org \
    --cc=darnok@68k.org \
    --cc=dwm@austin.ibm.com \
    --cc=dwm@enoyolf.org \
    --cc=greg@kroah.com \
    --cc=hpa@zytor.com \
    --cc=konradr@linux.vnet.ibm.com \
    --cc=konradr@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=mike.anderson@us.ibm.com \
    --cc=pjones@redhat.com \
    --cc=randy.dunlap@oracle.com \
    --subject='Re: [PATCH] Add iSCSI iBFT support (v0.4.6)' \
    /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

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).