svn commit: r238907 - projects/calloutng/sys/kern

[Konstantin Belousov]

On Fri, Sep 07, 2012 at 11:35:15PM +0100, Attilio Rao wrote:
> On Mon, Jul 30, 2012 at 9:39 PM, Attilio Rao <attilio at freebsd.org> wrote:
> > On Mon, Jul 30, 2012 at 4:31 PM, Andriy Gapon <avg at freebsd.org> wrote:
> >> on 30/07/2012 18:04 Attilio Rao said the following:
> >>> On 7/30/12, Andriy Gapon <avg at freebsd.org> wrote:
> >>>> on 30/07/2012 17:56 Attilio Rao said the following:
> >>>>> More explicitly, I think such combination TDP_NOSLEEPING +
> >>>>> TDP_NOBLOCKING (name invented) should be set on entering the interrupt
> >>>>> context, not only related to this part of callouts. This would be a
> >>>>> very good help for catching buggy situations.
> >>>>
> >>>> Something very tangential.  I think it would also be nice to check if a
> >>>> thread has
> >>>> any(?) locks held when returning to userland.
> >>>
> >>> This happens already for INVARIANTS case, with td_locks counters.
> >>> In the !INVARIANTS case, this doesn't happen because you don't want to
> >>> add the burden to bump td_locks for the fast case and I think it is a
> >>> good approach.
> >>
> >> Ah, I missed that, thank you.
> >> BTW, it seems that td_locks is checked twice in normal syscallret() path: once in
> >> syscallret() itself and then in userret().  On this note, would it make sense to
> >> move the whole nine yards of asserts from syscallret() to userret()?
> >> I mean it might make sense to have those checks (td_critnest, td_pflags) in other
> >> paths to userland.
> >
> > Nice catch.
> > The checks were added to syscallret() in r208453. While this is fine,
> > I think that putting them in userret() may give them more exposure and
> > cover also cases like traps which are not covered right now.
> > If you want to make a patch that moves these conditions in userret()
> > I'd be in favor of it.
> 
> More specifically, what do you think about this patch?:
> http://www.freebsd.org/~attilio/userret_diag.patch
> 
> Of course I moved the XEN par too before the checks.
> The patch survived to few consecutive and parallel buildworlds, FWIW.

At least in fork_return(), the last assert which checks that Giant is not
held, is no longer needed.

I had similar thought from the time when I added TDP_NOFAULTING check to the
syscallret(), but the loss of the syscall name in the panic output always
stopped me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-projects/attachments/20120908/653a7d46/attachment.pgp