This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Adding madvise function in cygwin with a #define


I have read cygwin "posix_madvise" code, that is in "mmap.cc" file and
"madvise" linux code and posix especification:
  *  CYGWIN => mmap.cc file:
http://cygwin.com/cgi-bin/cvsweb.cgi/~checkout~/src/winsup/cygwin/mmap.cc?cvsroot=src
  *  LINUX => madvise man:
http://www.kernel.org/doc/man-pages/online/pages/man2/madvise.2.html
                  => madvise source:
http://lxr.linux.no/linux+v2.6.37/mm/madvise.c
  *  POSIX => posix_madvise specification:
http://pubs.opengroup.org/onlinepubs/009695399/functions/posix_madvise.html

Your argument "madvise is more powerful than posix_madvise" in linux
is correct, the diference about standard "posix_madvise" and linux
"madvise" are the number of advices that admit in "int advice"
parameters, Linux implementation admits 10 options but standard Posix
only defines 5.

  -  Standard posix_madvise:
POSIX_MADV_NORMAL, POSIX_MADV_SEQUENTIAL, POSIX_MADV_RANDOM,
POSIX_MADV_WILLNEED, POSIX_MADV_DONTNEED

  -  Linux madvise:
MADV_NORMAL, MADV_SEQUENTIAL, MADV_RANDOM, MADV_REMOVE, MADV_WILLNEED,
MADV_DONTNEED, MADV_MERGEABLE, MADV_UNMERGEABLE, MADV_DOFORK,
MADV_DONTFORK

But, in Linux, internal posix_madvise and madvise implementation is the same.

The main problem is that current Cygwin "posix_madvise" implementation
is more or less a "dummy" function. Its functionality only consists
only in testing input parameters:

1) Checks "advice" and "len" parameters are correct. Return EINVAL if
they are incorrect.
2) Checks memory area. Return ENOMEM if requested memory is incorrect.
3) Do nothing (there is a comment:  /* Eventually do nothing. */  and return 0 )

After "advice" parameter checking, the function does not use "advice"
parameter anywhere:

extern "C" int
posix_madvise (void *addr, size_t len, int advice)
{
  /* Check parameters. */
  if (advice < POSIX_MADV_NORMAL || advice > POSIX_MADV_DONTNEED
      || !len)
    return EINVAL;

  /* Check requested memory area. */
  MEMORY_BASIC_INFORMATION m;
  char *p = (char *) addr;
  char *endp = p + len;
  while (p < endp)
    {
      if (!VirtualQuery (p, &m, sizeof m) || m.State == MEM_FREE)
	return ENOMEM;
      p = (char *) m.BaseAddress + m.RegionSize;
    }

  /* Eventually do nothing. */
  return 0;
}


In Cygwin, the current standard "posix_madvise" does nothing with any
of the posix defined "advices"

So in my opinion it has no sense to have a non working "posix_madvise"
function inside Cygwin, and after that, to reject to add a "madvise"
implementation using this non working "posix_madvise" saying that this
madvise will not work as espected in Linux, but now current
"posix_madvise" also doesnot work as espected in Linux or in Posix
standard.

So if my workaround "it is not a panacea for all clients of madvise",
I think it is even worse to have a "posix_madvise" that makes nothing
and return zero if input parameters are OK.


So in my opinion, my fix should be as good as current functionality:

1) define madvise as posix_madvise alias:
#define madvise posix_madvise

2) define madvise constants as alias of posix_madvise constants:
#define MADV_NORMAL POSIX_MADV_NORMAL
#define MADV_SEQUENTIAL POSIX_MADV_SEQUENTIAL
#define MADV_RANDOM POSIX_MADV_RANDOM
#define MADV_WILLNEED POSIX_MADV_WILLNEED
#define MADV_DONTNEED POSIX_MADV_DONTNEED



2011/1/11 Eric Blake:
> On 01/11/2011 04:13 AM, jdzstz - gmail dot com wrote:
>> Testing a linux application that uses "madvise", varnish cache, I have
>> realized that in cygwin doesnot exists this function but exists the
>> alternative "posix_madvise".
>>
>> Adding inside configure.ac script:
>> + ? ? ? ? ? ? AC_DEFINE([madvise], [posix_madvise], [In CYGWIN, madvise function
>> is not defined, instead we use posix_madvise])
>> + ? ? ? ? ? ? AC_DEFINE([MADV_RANDOM], [POSIX_MADV_RANDOM], [In CYGWIN,
>> MADV_RANDOM define is not defined, instead we use POSIX_MADV_RANDOM])
>
> madvise is more powerful than posix_madvise; while this workaround may
> have let you get varnish working, it is not a panacea for all clients of
> madvise. ?You are better off filing a bug report against varnish that
> they should either use the POSIX interface, or add a configure-time
> check for the presence of madvise.
>
>> I think it could be interesting adding this defines to cygwin headers,
>> at the end of <sys/mman.h> file
>>
>> #define madvise posix_madvise
>> #define MADV_NORMAL POSIX_MADV_NORMAL
>> #define MADV_SEQUENTIAL POSIX_MADV_SEQUENTIAL
>> #define MADV_RANDOM POSIX_MADV_RANDOM
>> #define MADV_WILLNEED POSIX_MADV_WILLNEED
>> #define MADV_DONTNEED POSIX_MADV_DONTNEED
>
> While these defines may make sense _once madvise is implemented_ in
> cygwin, I don't want them any sooner. ?In other words, we need a cygwin
> port of madvise before we pollute the namespace with MADV_* constants.
> That said, since Linux supports madvise and cygwin tries to emulate
> Linux, a patch to implement madvise() is more than welcome.
>
> --
> Eric Blake ? eblake@redhat.com ? ?+1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>
>

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]