X-Recipient: archive-cygwin AT delorie DOT com
X-SWARE-Spam-Status: No, hits=-1.7 required=5.0	tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST,TW_DZ,T_TO_NO_BRKTS_FREEMAIL
X-Spam-Check-By: sourceware.org
MIME-Version: 1.0
In-Reply-To: <4D2C6AE0.1000703@redhat.com>
References: <AANLkTimioT4OZ7CfW1Kh7R1nWv7uKz6v_3w=Hcgv-J6D AT mail DOT gmail DOT com>	<4D2C6AE0 DOT 1000703 AT redhat DOT com>
Date: Tue, 11 Jan 2011 20:12:23 +0100
Message-ID: <AANLkTin_qu-uidFWgas31-v4MwWqLbi75svwtO-cUKoZ@mail.gmail.com>
Subject: Re: Adding madvise function in cygwin with a #define
From: jdzstz - gmail dot com <jdzstz AT gmail DOT com>
To: cygwin AT cygwin DOT com
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
X-IsSubscribed: yes
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Precedence: bulk
List-Id: <cygwin.cygwin.com>
List-Unsubscribe: <mailto:cygwin-unsubscribe-archive-cygwin=delorie DOT com AT cygwin DOT com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com

I have read cygwin "posix_madvise" code, that is in "mmap.cc" file and
"madvise" linux code and posix especification:
  *  CYGWIN =3D> mmap.cc file:
http://cygwin.com/cgi-bin/cvsweb.cgi/~checkout~/src/winsup/cygwin/mmap.cc?c=
vsroot=3Dsrc
  *  LINUX =3D> madvise man:
http://www.kernel.org/doc/man-pages/online/pages/man2/madvise.2.html
                  =3D> madvise source:
http://lxr.linux.no/linux+v2.6.37/mm/madvise.c
  *  POSIX =3D> 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 sam=
e.

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 retur=
n 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 =3D (char *) addr;
  char *endp =3D p + len;
  while (p < endp)
    {
      if (!VirtualQuery (p, &m, sizeof m) || m.State =3D=3D MEM_FREE)
	return ENOMEM;
      p =3D (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:
>> + =A0 =A0 =A0 =A0 =A0 =A0 AC_DEFINE([madvise], [posix_madvise], [In CYGW=
IN, madvise function
>> is not defined, instead we use posix_madvise])
>> + =A0 =A0 =A0 =A0 =A0 =A0 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. =A0You 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. =A0In 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 =A0 eblake AT redhat DOT com =A0 =A0+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