1. 14 Sep, 2017 1 commit
    • Michal Hocko's avatar
      mm: treewide: remove GFP_TEMPORARY allocation flag · 0ee931c4
      Michal Hocko authored
      GFP_TEMPORARY was introduced by commit e12ba74d ("Group short-lived
      and reclaimable kernel allocations") along with __GFP_RECLAIMABLE.  It's
      primary motivation was to allow users to tell that an allocation is
      short lived and so the allocator can try to place such allocations close
      together and prevent long term fragmentation.  As much as this sounds
      like a reasonable semantic it becomes much less clear when to use the
      highlevel GFP_TEMPORARY allocation flag.  How long is temporary? Can the
      context holding that memory sleep? Can it take locks? It seems there is
      no good answer for those questions.
      
      The current implementation of GFP_TEMPORARY is basically GFP_KERNEL |
      __GFP_RECLAIMABLE which in itself is tricky because basically none of
      the existing caller provide a way to reclaim the allocated memory.  So
      this is rather misleading and hard to evaluate for any benefits.
      
      I have checked some random users and none of them has added the flag
      with a specific justification.  I suspect most of them just copied from
      other existing users and others just thought it might be a good idea to
      use without any measuring.  This suggests that GFP_TEMPORARY just
      motivates for cargo cult usage without any reasoning.
      
      I believe that our gfp flags are quite complex already and especially
      those with highlevel semantic should be clearly defined to prevent from
      confusion and abuse.  Therefore I propose dropping GFP_TEMPORARY and
      replace all existing users to simply use GFP_KERNEL.  Please note that
      SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic and
      so they will be placed properly for memory fragmentation prevention.
      
      I can see reasons we might want some gfp flag to reflect shorterm
      allocations but I propose starting from a clear semantic definition and
      only then add users with proper justification.
      
      This was been brought up before LSF this year by Matthew [1] and it
      turned out that GFP_TEMPORARY really doesn't have a clear semantic.  It
      seems to be a heuristic without any measured advantage for most (if not
      all) its current users.  The follow up discussion has revealed that
      opinions on what might be temporary allocation differ a lot between
      developers.  So rather than trying to tweak existing users into a
      semantic which they haven't expected I propose to simply remove the flag
      and start from scratch if we really need a semantic for short term
      allocations.
      
      [1] http://lkml.kernel.org/r/20170118054945.GD18349@bombadil.infradead.org
      
      [akpm@linux-foundation.org: fix typo]
      [akpm@linux-foundation.org: coding-style fixes]
      [sfr@canb.auug.org.au: drm/i915: fix up]
        Link: http://lkml.kernel.org/r/20170816144703.378d4f4d@canb.auug.org.au
      Link: http://lkml.kernel.org/r/20170728091904.14627-1-mhocko@kernel.orgSigned-off-by: default avatarMichal Hocko <mhocko@suse.com>
      Signed-off-by: default avatarStephen Rothwell <sfr@canb.auug.org.au>
      Acked-by: default avatarMel Gorman <mgorman@suse.de>
      Acked-by: default avatarVlastimil Babka <vbabka@suse.cz>
      Cc: Matthew Wilcox <willy@infradead.org>
      Cc: Neil Brown <neilb@suse.de>
      Cc: "Theodore Ts'o" <tytso@mit.edu>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      0ee931c4
  2. 21 Apr, 2016 3 commits
  3. 21 Jan, 2016 1 commit
    • James Bottomley's avatar
      string_helpers: fix precision loss for some inputs · 564b026f
      James Bottomley authored
      It was noticed that we lose precision in the final calculation for some
      inputs.  The most egregious example is size=3000 blk_size=1900 in units
      of 10 should yield 5.70 MB but in fact yields 3.00 MB (oops).
      
      This is because the current algorithm doesn't correctly account for
      all the remainders in the logarithms.  Fix this by doing a correct
      calculation in the remainders based on napier's algorithm.
      
      Additionally, now we have the correct result, we have to account for
      arithmetic rounding because we're printing 3 digits of precision.  This
      means that if the fourth digit is five or greater, we have to round up,
      so add a section to ensure correct rounding.  Finally account for all
      possible inputs correctly, including zero for block size.
      
      Fixes: b9f28d86Signed-off-by: default avatarJames Bottomley <JBottomley@Odin.com>
      Reported-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
      Cc: <stable@vger.kernel.org>	[delay until after 4.4 release]
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      564b026f
  4. 18 Sep, 2015 1 commit
    • Vitaly Kuznetsov's avatar
      lib/string_helpers.c: fix infinite loop in string_get_size() · 62bef58a
      Vitaly Kuznetsov authored
      Some string_get_size() calls (e.g.:
       string_get_size(1, 512, STRING_UNITS_10, ..., ...)
       string_get_size(15, 64, STRING_UNITS_10, ..., ...)
      ) result in an infinite loop. The problem is that if size is equal to
      divisor[units]/blk_size and is smaller than divisor[units] we'll end
      up with size == 0 when we start doing sf_cap calculations:
      
      For string_get_size(1, 512, STRING_UNITS_10, ..., ...) case:
         ...
         remainder = do_div(size, divisor[units]); -> size is 0, remainder is 1
         remainder *= blk_size; -> remainder is 512
         ...
         size *= blk_size; -> size is still 0
         size += remainder / divisor[units]; -> size is still 0
      
      The caller causing the issue is sd_read_capacity(), the problem was
      noticed on Hyper-V, such weird size was reported by host when scanning
      collides with device removal.  This is probably a separate issue worth
      fixing, this patch is intended to prevent the library routine from
      infinite looping.
      Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
      Acked-by: default avatarJames Bottomley <JBottomley@Odin.com>
      Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
      Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
      Cc: "K. Y. Srinivasan" <kys@microsoft.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      62bef58a
  5. 10 Sep, 2015 2 commits
  6. 15 Apr, 2015 2 commits
    • Rasmus Villemoes's avatar
      lib/string_helpers.c: change semantics of string_escape_mem · 41416f23
      Rasmus Villemoes authored
      The current semantics of string_escape_mem are inadequate for one of its
      current users, vsnprintf().  If that is to honour its contract, it must
      know how much space would be needed for the entire escaped buffer, and
      string_escape_mem provides no way of obtaining that (short of allocating a
      large enough buffer (~4 times input string) to let it play with, and
      that's definitely a big no-no inside vsnprintf).
      
      So change the semantics for string_escape_mem to be more snprintf-like:
      Return the size of the output that would be generated if the destination
      buffer was big enough, but of course still only write to the part of dst
      it is allowed to, and (contrary to snprintf) don't do '\0'-termination.
      It is then up to the caller to detect whether output was truncated and to
      append a '\0' if desired.  Also, we must output partial escape sequences,
      otherwise a call such as snprintf(buf, 3, "%1pE", "\123") would cause
      printf to write a \0 to buf[2] but leaving buf[0] and buf[1] with whatever
      they previously contained.
      
      This also fixes a bug in the escaped_string() helper function, which used
      to unconditionally pass a length of "end-buf" to string_escape_mem();
      since the latter doesn't check osz for being insanely large, it would
      happily write to dst.  For example, kasprintf(GFP_KERNEL, "something and
      then %pE", ...); is an easy way to trigger an oops.
      
      In test-string_helpers.c, the -ENOMEM test is replaced with testing for
      getting the expected return value even if the buffer is too small.  We
      also ensure that nothing is written (by relying on a NULL pointer deref)
      if the output size is 0 by passing NULL - this has to work for
      kasprintf("%pE") to work.
      
      In net/sunrpc/cache.c, I think qword_add still has the same semantics.
      Someone should definitely double-check this.
      
      In fs/proc/array.c, I made the minimum possible change, but longer-term it
      should stop poking around in seq_file internals.
      
      [andriy.shevchenko@linux.intel.com: simplify qword_add]
      [andriy.shevchenko@linux.intel.com: add missed curly braces]
      Signed-off-by: default avatarRasmus Villemoes <linux@rasmusvillemoes.dk>
      Acked-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      41416f23
    • Rasmus Villemoes's avatar
      lib/string_helpers.c: refactor string_escape_mem · 3aeddc7d
      Rasmus Villemoes authored
      When printf is given the format specifier %pE, it needs a way of obtaining
      the total output size that would be generated if the buffer was large
      enough, and string_escape_mem doesn't easily provide that.  This is a
      refactorization of string_escape_mem in preparation of changing its
      external API to provide that information.
      
      The somewhat ugly early returns and subsequent seemingly redundant
      conditionals are to make the following patch touch as little as possible
      in string_helpers.c while still preserving the current behaviour of never
      outputting partial escape sequences.  That behaviour must also change for
      %pE to work as one expects from every other printf specifier.
      Signed-off-by: default avatarRasmus Villemoes <linux@rasmusvillemoes.dk>
      Acked-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      3aeddc7d
  7. 10 Apr, 2015 1 commit
    • James Bottomley's avatar
      sd, mmc, virtio_blk, string_helpers: fix block size units · b9f28d86
      James Bottomley authored
      The current string_get_size() overflows when the device size goes over
      2^64 bytes because the string helper routine computes the suffix from
      the size in bytes.  However, the entirety of Linux thinks in terms of
      blocks, not bytes, so this will artificially induce an overflow on very
      large devices.  Fix this by making the function string_get_size() take
      blocks and the block size instead of bytes.  This should allow us to
      keep working until the current SCSI standard overflows.
      
      Also fix virtio_blk and mmc (both of which were also artificially
      multiplying by the block size to pass a byte side to string_get_size()).
      
      The mathematics of this is pretty simple:  we're taking a product of
      size in blocks (S) and block size (B) and trying to re-express this in
      exponential form: S*B = R*N^E (where N, the exponent is either 1000 or
      1024) and R < N.  Mathematically, S = RS*N^ES and B=RB*N^EB, so if RS*RB
      < N it's easy to see that S*B = RS*RB*N^(ES+EB).  However, if RS*BS > N,
      we can see that this can be re-expressed as RS*BS = R*N (where R =
      RS*BS/N < N) so the whole exponent becomes R*N^(ES+EB+1)
      
      [jejb: fix incorrect 32 bit do_div spotted by kbuild test robot <fengguang.wu@intel.com>]
      Acked-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
      Reviewed-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarJames Bottomley <JBottomley@Odin.com>
      b9f28d86
  8. 13 Feb, 2015 3 commits
  9. 14 Oct, 2014 2 commits
  10. 07 Aug, 2014 1 commit
  11. 01 May, 2013 1 commit
  12. 29 May, 2012 1 commit
  13. 07 Mar, 2012 1 commit
  14. 23 Oct, 2008 1 commit
    • H. Peter Anvin's avatar
      [SCSI] lib: string_get_size(): don't hang on zero; no decimals on exact · a8659597
      H. Peter Anvin authored
      We would hang forever when passing a zero to string_get_size().
      Furthermore, string_get_size() would produce decimals on a value small
      enough to be exact.  Finally, a few formatting issues are inconsistent
      with standard SI style guidelines.
      
      - If the value is less than the divisor, skip the entire rounding
        step.  This prints out all small values including zero as integers,
        without decimals.
      - Add a space between the value and the symbol for the unit,
        consistent with standard SI practice.
      - Lower case k in kB since we are talking about powers of 10.
      - Finally, change "int" to "unsigned int" in one place to shut up a
        gcc warning when compiling the code out-of-kernel for testing.
      Signed-off-by: default avatarH. Peter Anvin <hpa@zytor.com>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      a8659597
  15. 03 Oct, 2008 1 commit