Core ReactOS
  1. Core ReactOS
  2. CORE-12711

Special Pool: support "MmSpecialPoolTag = '*'"

    Details

    • Type: Improvement Improvement
    • Status: Resolved Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Fix Version/s: 0.4.5
    • Component/s: NTCore
    • Labels:

      Description

      (Wiki) Debugging > Special Pool
      Wiki r36450 (by Thomas Faber) added

      • To enable Special Pool for all allocations, set <code>MmSpecialPoolTag</code> to <code>'*'</code>.

      But
      Log of .../special.c never had related code:

      75 	BOOLEAN
      76 	NTAPI
      77 	MmUseSpecialPool(SIZE_T NumberOfBytes, ULONG Tag)
      78 	{
      79 	/* Special pool is not suitable for allocations bigger than 1 page */
      80 	if (NumberOfBytes > (PAGE_SIZE - sizeof(POOL_HEADER)))
      81 	return FALSE;
      82 	
      83 	return Tag == MmSpecialPoolTag;
      84 	}
      

      Do we want to update the wiki?
      Or to actually support '*' value?

        Activity

        Hide
        Serge Gautherie
        added a comment - - edited

        While there:

        MmSpecialPoolTag search

        2)
        Shouldn't MmSpecialPoolTag be initialized (to 0)?

        3)
        Why do we need to support both 0 and -1 to disable Special Pool?

        Show
        Serge Gautherie
        added a comment - - edited While there: MmSpecialPoolTag search 2) Shouldn't MmSpecialPoolTag be initialized (to 0 )? 3) Why do we need to support both 0 and -1 to disable Special Pool?
        Hide
        Thomas Faber
        added a comment -

        Global variables that don't have an initializer are automatically zeroed, so an explicit = 0 isn't necessary.
        We absolutely should check for '*' and take that to mean everything. I thought I had implemented that, but apparently did not.

        All of this mimics Windows's behavior, which I believe is why -1 is also recognized. Windows actually supports full wildcard matching (e.g. '*b?' covers 'dcba', 'bA') which we obviously also still lack.

        Show
        Thomas Faber
        added a comment - Global variables that don't have an initializer are automatically zeroed, so an explicit = 0 isn't necessary. We absolutely should check for '*' and take that to mean everything. I thought I had implemented that, but apparently did not. All of this mimics Windows's behavior, which I believe is why -1 is also recognized. Windows actually supports full wildcard matching (e.g. '*b?' covers 'dcba', 'bA') which we obviously also still lack.
        Hide
        Mark Jansen
        added a comment -

        Serge Gautherie: There is no need for the define:

        • If special pool is not enabled, this code will never be called.
        • If special pool is enabled, there are far worse hits for performance than one check per allocation for '*'
        Show
        Mark Jansen
        added a comment - Serge Gautherie : There is no need for the define: If special pool is not enabled, this code will never be called. If special pool is enabled, there are far worse hits for performance than one check per allocation for '*'
        Hide
        Serge Gautherie
        added a comment -

        1) '*':
        CORE-12711_MmSpecialPoolTag_SupportAllTag.patch

        MmSpecialPoolTag: Document (special) values.
        MmUseSpecialPool(): Support "MmSpecialPoolTag == '*'" (= all pools).
        

        2) "= 0":
        Right!

        3) 0, -1:
        I assumed so, I'll take your word for it.

        4) "full wildcard matching":
        I filed CORE-12714.

        Show
        Serge Gautherie
        added a comment - 1) '*': CORE-12711_MmSpecialPoolTag_SupportAllTag.patch MmSpecialPoolTag: Document (special) values. MmUseSpecialPool(): Support "MmSpecialPoolTag == '*'" (= all pools). 2) "= 0": Right! 3) 0, -1: I assumed so, I'll take your word for it. 4) "full wildcard matching": I filed CORE-12714 .
        Hide
        Serge Gautherie
        added a comment - - edited

        Thomas Faber,

        • Thanks for the commit
        • Could you (re)add "Fix Version/s 0.4.4", as this fix was copied to that branch.
        • What about the two comment/documentation parts?

        PS:
        Ftr, -1 is a bit odd as a ULONG value. Probably meant to be ~0...

        Show
        Serge Gautherie
        added a comment - - edited Thomas Faber , Thanks for the commit Could you (re)add "Fix Version/s 0.4.4", as this fix was copied to that branch. What about the two comment/documentation parts? PS: Ftr, -1 is a bit odd as a ULONG value. Probably meant to be ~0 ...
        Hide
        Thomas Faber
        added a comment -

        I don't think it was added to the real 0.4.4, only the FOSDEM version.

        The code doesn't need documentation – it belongs on the wiki.

        Show
        Thomas Faber
        added a comment - I don't think it was added to the real 0.4.4, only the FOSDEM version. The code doesn't need documentation – it belongs on the wiki.
        Hide
        Serge Gautherie
        added a comment -

        I don't think it was added to the real 0.4.4, only the FOSDEM version.

        Ah, right. (Confusing.)

        The code doesn't need documentation – it belongs on the wiki.

        (I would say "both", but I won't argue.)

        Show
        Serge Gautherie
        added a comment - I don't think it was added to the real 0.4.4, only the FOSDEM version. Ah, right. (Confusing.) The code doesn't need documentation – it belongs on the wiki. (I would say "both", but I won't argue.)

          People

          • Assignee:
            Thomas Faber
            Reporter:
            Serge Gautherie
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: