lkml.org 
[lkml]   [2009]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch 06/12] Use the new wrapper routines to access debug registers in process/thread code
    On Fri, May 29, 2009 at 04:07:07PM +0200, Frédéric Weisbecker wrote:
    > 2009/5/29 K.Prasad <prasad@linux.vnet.ibm.com>:
    > > On Fri, May 29, 2009 at 12:49:03PM +0200, Frederic Weisbecker wrote:
    > >> On Fri, May 29, 2009 at 02:31:46PM +0530, K.Prasad wrote:
    > >> > On Thu, May 28, 2009 at 04:42:38PM +1000, David Gibson wrote:
    > >> > > On Mon, May 11, 2009 at 05:23:44PM +0530, K.Prasad wrote:
    > >> > > > From: Alan Stern <stern@rowland.harvard.edu>
    > >> > > >
    > >> > > > This patch enables the use of abstract debug registers in
    > >> > > > process-handling routines.
    > >> > >
    > >> > > [snip]
    > >> > > >
    > >> > > > +       p->thread.io_bitmap_ptr = NULL;
    > >> > >
    > >> > > Why is manipulating the io_bitmap_ptr relevant to debug register
    > >> > > handling?
    > >> >
    > >> > I *re-read* the patch but was unable to find how this change had sneaked
    > >> > in. It shouldn't be there although it is harmless.
    > >>
    > >>
    > >> When I reviewed this patch, I also ended stucked on it.
    > >> But actually I guess I found the sense, this is only for
    > >> convenience.
    > >>
    > >> Look at the current copy_thread() in arch/x86/kernel/process32.c
    > >>
    > >> If p->thread.io_bitmap_ptr fails to be duplicated, we set
    > >> p->thread.io_bitmap_max = 0 and return -ENOMEM
    > >>
    > >> Now look at the patch.
    > >> If we fail to copy the hardware thread virtual registers we
    > >> want to exit with io_bitmap_ptr = NULL
    > >> If we fail to copy the io_bitmap, we want to free the breakpoint
    > >> and exit.
    > >> If we fail further, we want to free breakpoints and io_bitmap_ptr
    > >>
    > >> The out section then tries to:
    > >>
    > >> -free the breakpoints
    > >> -free p->thread.io_bitmap_ptr
    > >>
    > >>
    > >> So it's important to set io_bitmap_ptr to NULL so that
    > >> we know whether we have to release it or not.
    > >>
    > >>
    > >
    > > aah...yes. It tricked me! It is needed to bring the desired
    > > error-return behaviour of copy_thread(). Please ignore this patch (the
    > > updation of the comments can be brought in through a separate
    > > enhancement patch...see below).
    >
    >
    > Ok.
    >
    >
    > >> > Hi Frederic,
    > >> >     I am attaching a new version of this patch 06/12 that:
    > >> >
    > >> > - removes the line that assigns NULL to "p->thread.io_bitmap_ptr"
    > >>
    > >>
    > >> Dangerous. Unless p->thread.io_bitmap_ptr is already zeroed out
    > >> at this stage?
    > >>
    > >>
    > >> > - Updates the comment in __switch_to() function which was stale (was
    > >> >   relevant when 'last_debugged_task' was used to detect lazy debug
    > >> >   register switching).
    > >> >
    > >> > Kindly integrate this version in lieu of the older version sent here:
    > >> > http://lkml.org/lkml/2009/5/21/149.
    > >>
    > >>
    > >> Ok. Well it would be nice if you resend the whole series actually :)
    > >> Do you have another fix pending?
    > >>
    > >> Thanks!
    > >>
    > >
    > > In the process of responding to David Gibson's comments I agreed to
    > > a few minor/cosmetic changes - say like updation of comments, renaming
    > > functions or variables, etc.
    > >
    > > Given that the patchset is on the verge of integration into -tip tree, I
    > > would prefer them to be done through a separate patch (on -tip) for
    > > enhancement. Kindly let me know what you think about proceeding this
    > > way.
    >
    >
    > Well, indeed it would be easier for you to start iterating with a merged base.
    > But I would prefer to pull-request the patchset to Ingo once the pending fixes
    > are sent.
    >
    > So to start the integration of this, I can apply the current patches in my tree,
    > based on tracing/core. And once you have the minor fixes addressing David's
    > comments, I also apply them and send the whole to Ingo.
    >
    > Ok?
    > But still could you resend me the whole patchset you have, including
    > the fixes already posted, so that I don't mess up through several versions.
    >
    > And I guess I could apply them very soon.
    >
    > Thanks.
    >

    Hi Frederic,
    I have re-sent the patchset which includes some of the changes
    that were made on them since their previous posting.

    Please find the new patchset addressed to you starting here:
    http://lkml.org/lkml/2009/5/30/67.

    I would like to bring in some of the enhancements I agreed to David,
    through a separate patchset after its integration into -tip tree. Just
    as you mentioned above, it should be much easier to test once there's
    a merged base.

    Thanks,
    K.Prasad

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-05-30 13:03    [W:0.046 / U:5.300 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site